[Lldb-commits] [PATCH]race condition calling PushProcessIOHandler
Matthew Gardiner
mg11 at csr.com
Wed Jul 30 22:51:42 PDT 2014
Hi Shawn,
Yes, the condition variable paradigm can initially be a little tricky to
grasp. Have a look at something like
http://pages.cs.wisc.edu/~remzi/OSTEP/threads-cv.pdf, first. I think the
text on page on page 14 sums up the concept best. You should probably
study it's operation in a simple test program first.
The concept, IIRC, is as follows: One (or more) threads needs to wait
for a condition. I think in our case this is the main command thread,
and it needs to wait till worker thread has completed and has pushed the
IOHandler. Instead of having the main thread sleeping a fixed interval,
you let it sleep (possibly for a infinite timeout) and wake when a
condition occurs (e.g. a flag is set, or variable exceeds a particular
value). The other thread (e.g. the one pushing the IOHandler) then sets
the signal once the condition has arisen. The point is, we wait for a
signal, *not* a timeout - the timeout is just (sometimes) there for if
things go wrong, e.g. the condition is never met - and in that case this
timeout/error condition would need to be dealt with (by the waiting
thread) correctly (e.g. an error message preceding the printing of the
prompt).
Greg Clayton's reply to you suggests that you use
"lldb/Host/Predicate.h". I've not used this myself, but I took a quick
look and it looks like it wraps up the above idea. I *think* that using
this, the main thread (the one waiting) will call one of the Predicate
class's Waitxxx functions, and the other thread will call on of the
Setxxx when it's ok for the waiter to wake.
I'd suggest you get your head round conditional variables first, then
check out using "lldb/Host/Predicate.h" in the lldb code.
Hope this helps! Keep us posted.
Matt
Shawn Best wrote:
> I have reworked the patch to use std::condition_variable. This
> particular sync mechanism was new to me, I hope I used it correctly.
> Is it portable across all target platforms/compilers? I tested on
> linux and OSX.
>
> The timeout is pretty small (1ms) but seems ample based on the
> measurements I made.
>
>
> On Tue, Jul 29, 2014 at 9:58 PM, Matthew Gardiner <mg11 at csr.com
> <mailto:mg11 at csr.com>> wrote:
>
> Cool, let us know how you get on!
> Matt
>
> Shawn Best wrote:
>
> 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> <mailto: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>
> <mailto: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>
> <http://www.csr.com>. Keep up to date with CSR on our
> technical
> blog, www.csr.com/blog <http://www.csr.com/blog>
> <http://www.csr.com/blog>, CSR people
> blog, www.csr.com/people <http://www.csr.com/people>
> <http://www.csr.com/people>, YouTube,
> www.youtube.com/user/CSRplc
> <http://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>
> <http://www.facebook.com/pages/CSR/191038434253534>,
> or follow us
> on Twitter at www.twitter.com/CSR_plc
> <http://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>
> <http://www.aptx.com>.
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu <mailto:lldb-commits at cs.uiuc.edu>
> <mailto:lldb-commits at cs.uiuc.edu
> <mailto:lldb-commits at cs.uiuc.edu>>
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
>
>
>
>
More information about the lldb-commits
mailing list