[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

I think we should slow down a bit.

For consistency, and to minimize the number of ifdefs, I believe windows and non-windows paths should use as similar approaches as possible. That means I think the end state should be that the posix path also uses a fork+exec approach.

For me, it also seems natural that this should be the first step. While doing that, we can make sure to create abstractions that can be used to implement the windows behavior as well. I'm imagining some sort of a function or a class, which takes the target binary, its arguments, and a socket, and launches that binary with that socket. (we'll probably also need a similar function/class on the other end to receive that socket).

The function doesn't have to be terribly generic, but it ideally should encompass all of platform specific behavior, so that the surrounding code does not need to use ifdefs. (But, ideally, it should at least be slightly generic, so that one can write a unit test for this functionality).

(BTW, I'm OOO for the rest of the week, so I won't be able to review this in detail. Upon cursory inspection, the general idea makes sense, but we really need to figure out how to encapsulate this. I'm not sure if we really need the WriteWithTimeout functionality. The reason we don't have it so far is that pipes have a buffer, and unless we're writing a huge chunk of data, the write will never block anyway. There's nothing wrong with it in principle though, and if you do want to have it, it'd be best if it comes with in its own patch and with a test.)

https://github.com/llvm/llvm-project/pull/101283