[Lldb-commits] [PATCH]race condition calling PushProcessIOHandler

[Shawn Best]

Thanks for the feedback guys.

Studying the code, I had figured going with a straight int would in 
practice be most efficient and not run into multi-threaded problems, 
even if initially appearing a bit risky.  I will rework it to use a 
std::condition_variable.  That will be more robust and readable.

Shawn.

On 7/29/2014 10:53 AM, Zachary Turner wrote:
> Even better would be an std::condition_variable
>
>
> On Mon, Jul 28, 2014 at 10:30 PM, Matthew Gardiner <mg11 at csr.com 
> <mailto:mg11 at csr.com>> wrote:
>
>     Hi Shawn,
>
>     I use 64-bit linux and I see this issue a lot. It usually
>     manifests itself as the prompt just not being printed (or perhaps
>     it just gets overwritten) - regardless - I invoke a command, and I
>     don't see an (lldb) prompt when I should. So I'm well pleased that
>     you are looking at this!
>
>     Would it not be more robust to use a semaphore than usleep to
>     synchronise the problematic threads?
>
>     Although I've not looked too deeply into this particular issue,
>     whenever I've seen similar races, I found that it's almost
>     impossible to pick the right value when using a sleep command. A
>     semaphore, though, should always ensure the waiting thread will
>     wake precisely.
>
>     I'd be happy to help to test such a fix.
>
>     Matt
>
>
>     Shawn Best wrote:
>
>         Hi,
>
>         I have attached a patch which addresses 3 related race
>         conditions that cause the command line (lldb) prompt to get
>         displayed inappropriately and make it appear it is not working
>         correctly.  This issue can be seen on linux and FreeBSD.  I
>         can also artificailly induce the problem on OSX.
>
>         The issue happens when the command handler (in the main
>         thread) issues a command such as run, step or continue.  After
>         the command finishes initiating its action, it returns up the
>         call stack and goes back into the main command loop waiting
>         for user input.  Simultaneously, as the inferior process
>         starts up, the MonitorChildProcess thread picks up the change
>         and posts to the PrivateEvent thread.  HandePrivateEvent()
>         then calls PushProcessIOHandler() which will disable the
>         command IO handler and give the inferior control of the TTY.
>          To observe this on OSX, put a
>
>         usleep(100);
>
>         immediately prior the PushProcessIOHandler() in
>         HandlePrivateEvent.
>
>
>         My proposed solution is that after a 'run', 'step', or
>         'continue' command, insert a synchronization point and wait
>         until HandlePrivateEvent knows the inferior process is running
>         and has pushed the IO handler.  One context switch (<100us) is
>         usually all the time it takes on my machine.  As an additional
>         safety, I have a timeout (currently 1ms) so it will never hang
>         the main thread.
>
>         Any thoughts, or suggestions would be appreciated.
>
>         Regards,
>         Shawn.
>
>
>         To report this email as spam click here
>         <https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==>.
>
>
>
>         _______________________________________________
>         lldb-commits mailing list
>         lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>         http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
>
>
>
>     Member of the CSR plc group of companies. CSR plc registered in
>     England and Wales, registered number 4187346, registered office
>     Churchill House, Cambridge Business Park, Cowley Road, Cambridge,
>     CB4 0WZ, United Kingdom
>     More information can be found at www.csr.com <http://www.csr.com>.
>     Keep up to date with CSR on our technical blog, www.csr.com/blog
>     <http://www.csr.com/blog>, CSR people blog, www.csr.com/people
>     <http://www.csr.com/people>, YouTube, www.youtube.com/user/CSRplc
>     <http://www.youtube.com/user/CSRplc>, Facebook,
>     www.facebook.com/pages/CSR/191038434253534
>     <http://www.facebook.com/pages/CSR/191038434253534>, or follow us
>     on Twitter at www.twitter.com/CSR_plc
>     <http://www.twitter.com/CSR_plc>.
>     New for 2014, you can now access the wide range of products
>     powered by aptX at www.aptx.com <http://www.aptx.com>.
>     _______________________________________________
>     lldb-commits mailing list
>     lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140729/dde37760/attachment.html>