[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)

[Jonas Devlieghere via lldb-commits]

JDevlieghere wrote:

> That's okay. I thought it might not work out. Thanks for giving it a shot.
> 
> I feel bad about bringing this up after you did all the work, but there is one more thing going through my head. I now have a very awkward feeling about this because it seems like this is solving mostly the same problem that StreamAsynchronousIO (and Debugger::PrintAsync) was trying to solve. You mentioned that in the first version of the patch and said that it doesn't work in some cases, but it's not quite clear why is that the case. If the motivation is status line, then it sounds like it _might_ be possible to plumb the updates through the async streams.

The async streams buffer until they're flushed (or more commonly destroyed) and they delegate to the IOHandler instead of writing directly to the debugger's output or error stream. The problem is that while the IOHandler can protect the write, as is the case for `IOHandlerEditline` using the output mutex, nothing prevents anyone else from writing directly to the output stream, not using the asynchronous handler. If everyone uses the asynchronous streams, and every IOHandler would protect the write using the same mutex, then we could achieve the same. But that really files like working around the issue rather than addressing it (i.e. protecting the underlying streams), which is what this patch does. 

> I can imagine that it might not work for some reason, but then I think it's important to know why, and come up with some sort of an story for which of the two systems should be used for a particular use case. Or, (I am dreaming, I know) actually deprecate/remove the async IO system.

Yes, with this patch protecting the underlying streams, I don't think we _need_ the asynchronous streams and most uses could be converted to lock the stream. That said, there are still uses cases that benefit from the buffering, like running the expressions. As you pointed out, we don't want to lock the streams for the duration of running the expression. But maybe we should rename them to "BufferedOutputStream" but still remove the `PrintAsync` stuff that delegates to the IO handlers. 


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