[Lldb-commits] [PATCH] D48463: Prevent dead locking when calling PrintAsync

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D48463#1140861, @teemperor wrote:

> Adding Pavel because he wrote the PrintAsync code.
>
> Also @labath: Can you tell me what variables/functionality the `m_output_mutex` in Editline.cpp is supposed to shield? I don't see any documentation for that.
>
> The `m_output_mutex` name suggests its related to console output, but we actually take the lock also when reading *input*. Especially in `EditLine::GetLine` we take a guard on the lock but then somehow unlock the guarded mutex from inside  `Editline::GetCharacter` that we call afterwards (which completely breaks this patch):


If I remember correctly (it was a long time ago), the idea was behind this was to make sure that nobody prints anything to stdout while libedit is playing around with it. That's why it's called "output" mutex. The thing that this (admittedly strange) locking strategy achieves is that the the only time the `PrintAsync` function can write to stdout is while we are blocked waiting for input, as that's the place we can be sure we aren't writing to stdout -- before, or after that, we could be in libedit code and it could mess with stdout to write it's prompt or whatever. IIRC, PrintAsync should eventually end up taking the same mutex, and then depending on the current state of libedit machinery, do the "right thing" wrt. erasing the prompt et al.

Hmm.. Now that I have spelled this out, it sounds like this same pattern could be used to achieve your goal too -- while you're inside the completion handler, you also shouldn't be doing any funny things with stdout. Maybe the solution would be do just drop the output mutex while the running the completion code. Could you try if something like that would work?

> 
> 
>   // This mutex is locked by our caller (GetLine). Unlock it while we read a
>   // character (blocking operation), so we do not hold the mutex
>   // indefinitely. This gives a chance for someone to interrupt us. After
>   // Read returns, immediately lock the mutex again and check if we were
>   // interrupted.

This comment refers to the second purpose of the surrounding code, which is to make sure that any interrupt request is handled exactly once.


https://reviews.llvm.org/D48463