[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)

[Pavel Labath via lldb-commits]

labath wrote:

> > If this is only really used for a "only for few requests via the platform protocol", then why not make the CWD a property of the platform object? (Either through a virtual filesystem, or just by having it as a string, and resolving things explicitly)
> 
> It is possible to store an own FileSystem object in the platform handler, but it requires to update 80% of GDBRemoteCommunicationServerCommon.cpp and implement some behavior switch in inherited classes.

That does not worry me. In fact, I would say that if *all* we need to update is GDBRemoteCommunicationServerCommon and its subclasses, then we're pretty good.


> 
> I tried to minimize changes. I have added the new FileSystem::InitializePerThread() which is used only in GDBRemoteCommunicationServerPlatform and its base clases in case of multithreading. All other code uses the same FileSystem, nothing changed. FileSystem::InitializePerThread() uses the CWD of the app. So the behavior for the thread is the same as for a forked child process.
> 
> I don't see any other threads where FileSystem is used. `lldb-server platform` creates only one additional thread to monitor a child process. But it does not use any file system operations.
> 
> Anyway if FileSystem::InitializePerThread() was not called, any new thread uses the common app's FileSystem. It is safe.

I realize all that, but I still don't think it's a good tradeoff -- complicating one low-level library (which is pretty complicated on its own), for the sake of one very specific user. I can see how someone might view it differently and you're welcome to find those people and get them on board with your approach. I just don't think I'm going to be one of them.

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