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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to