corral icon indicating copy to clipboard operation
corral copied to clipboard

Allow use of lldb with corral

Open KittyMac opened this issue 4 years ago • 4 comments

The "run" command captures all input and output of the command being run, making it difficult to be used with dynamic tools such as lldb when debugging ponyc changes. Instead of altering how run works, I added an "exec" command which just execve's the process and gives everything over to the command being executed. This has allowed full access for working with lldb.

KittyMac avatar May 06 '20 21:05 KittyMac

Thanks @KittyMac,

Are there any tests that can be provided to verify that it works and most importantly that it won't get accidentally broken in the future?

SeanTAllen avatar May 06 '20 21:05 SeanTAllen

Ok, I completed a second, better attempt at a fix. There are two parts:

  1. Update ProcessMonitor in ponyc to allow conditional piping of stdin/stdout/stderr ProcessMonitor always pipes and always replaces stdin/stdout/stderr in the forked process. If we add the ability to allow the caller to say "don't do that", then the forked process (ie lldb in this case) has full access to stdin/stdout/stderr and works as normal.

https://github.com/KittyMac/ponyc/commit/eaf3e1e3c1d5e2739ff8101bef7f8f004f10325e

  1. In corral, for the "run" command have it use the new feature in (1) to all the target of the run command can retain normal stdin/stdout/stderr access.

https://github.com/KittyMac/corral/commit/180d5bef1d2a26f9a78dc45f567a6e50c8c64828

If this seems like a better approach I can make the relevant PRs. Please note I didn't implement the companion changes in _ProcessWindows(), as I'm not currently set up to test on Windows.

KittyMac avatar May 07 '20 04:05 KittyMac

@mfelsche you know the process monitor stuff best, can you have a look?

SeanTAllen avatar May 09 '20 18:05 SeanTAllen

I think it makes very much sense to allow a child process to "take over" the parents stdin/out/err descriptors.

I am also not sure, if we might get some unwanted interleaving between host and child, both accessing stdin/out/err and if there might be something to prevent this. What if we call a pony program like this: cat my_file.txt | pony_program and pony_program spawns a new process without piping. How will this behave? Who receives data from stdin? This should be tested.

Also we need to make clear that a ProcessNotify will not be able to communicate with the child process and will not receive any stdout/stderr from it. This needs to be made clear in the docs at least.

And I would love to have a different interface than 3 boolean flags. I would suggest to have a different constructor on ProcessMonitor, that does not pipe any std stream to or from the child. But this lacks the power to pipe any stream selectively. So, also not perfect. I don't want to delay any progress on this, so i'd also go with the suggested interface if there is no other cool suggestion popping up.

I also don't know the windows side of things, but i assume this should also work on windows. The CI will tell the truth here. And either me or @kulibali can help with windows testing.

mfelsche avatar May 18 '20 19:05 mfelsche