<div dir="ltr">oops, with the attachment this time.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 31, 2014 at 2:56 PM, Shawn Best <span dir="ltr"><<a href="mailto:sbest@blueshiftinc.com" target="_blank">sbest@blueshiftinc.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks everyone for the feedback. I have reworked the patch to use Predicate <bool>, it reads much cleaner now.<span class="HOEnZb"><font color="#888888"><br>
<div class="gmail_extra"><br></div><div class="gmail_extra">Shawn.<br></div></font></span><div><div class="h5"><div class="gmail_extra">
<br><div class="gmail_quote">On Wed, Jul 30, 2014 at 6:34 PM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
You will want to use a Predicate<bool> here in stead of what you have since it is exactly what we use a predicate for. The following:<br>
<br>
+ bool m_process_running_sync; // used with WaitForProcessRunning() synchronization<br>
+ std::condition_variable m_condition_process_running; // used with WaitForProcessRunning() synchronization<br>
+ std::mutex m_mutex_process_running; // used with WaitForProcessRunning() synchronization<br>
<br>
Is exactly what the Predicate class does: protect a value with a mutex and condition.<br>
<br>
The above code should be replaced with:<br>
<br>
Predicate<bool> m_process_running_sync;<br>
<br>
The API on Predicate should do what you want. See the header file at "lldb/Host/Predicate.h" and also look for other places that use this class to wait for a value to be equal to another value, or wait for a value to not be equal to something.<br>
<br>
Let me know when you have a patch that uses Predicate and we will look at that.<br>
<br>
Greg<br>
<div><div><br>
> On Jul 30, 2014, at 4:03 PM, Shawn Best <<a href="mailto:sbest@blueshiftinc.com" target="_blank">sbest@blueshiftinc.com</a>> wrote:<br>
><br>
> 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.<br>
><br>
> The timeout is pretty small (1ms) but seems ample based on the measurements I made.<br>
><br>
><br>
> On Tue, Jul 29, 2014 at 9:58 PM, Matthew Gardiner <<a href="mailto:mg11@csr.com" target="_blank">mg11@csr.com</a>> wrote:<br>
> Cool, let us know how you get on!<br>
> Matt<br>
><br>
> Shawn Best wrote:<br>
> Thanks for the feedback guys.<br>
><br>
> 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.<br>
><br>
> Shawn.<br>
><br>
> On 7/29/2014 10:53 AM, Zachary Turner wrote:<br>
> Even better would be an std::condition_variable<br>
><br>
><br>
> On Mon, Jul 28, 2014 at 10:30 PM, Matthew Gardiner <<a href="mailto:mg11@csr.com" target="_blank">mg11@csr.com</a> <mailto:<a href="mailto:mg11@csr.com" target="_blank">mg11@csr.com</a>>> wrote:<br>
><br>
> Hi Shawn,<br>
><br>
> I use 64-bit linux and I see this issue a lot. It usually<br>
> manifests itself as the prompt just not being printed (or perhaps<br>
> it just gets overwritten) - regardless - I invoke a command, and<br>
> I don't see an (lldb) prompt when I should. So I'm well pleased<br>
> that you are looking at this!<br>
><br>
> Would it not be more robust to use a semaphore than usleep to<br>
> synchronise the problematic threads?<br>
><br>
> Although I've not looked too deeply into this particular issue,<br>
> whenever I've seen similar races, I found that it's almost<br>
> impossible to pick the right value when using a sleep command. A<br>
> semaphore, though, should always ensure the waiting thread will<br>
> wake precisely.<br>
><br>
> I'd be happy to help to test such a fix.<br>
><br>
> Matt<br>
><br>
><br>
> Shawn Best wrote:<br>
><br>
> Hi,<br>
><br>
> I have attached a patch which addresses 3 related race<br>
> conditions that cause the command line (lldb) prompt to get<br>
> displayed inappropriately and make it appear it is not<br>
> working correctly. This issue can be seen on linux and<br>
> FreeBSD. I can also artificailly induce the problem on OSX.<br>
><br>
> The issue happens when the command handler (in the main<br>
> thread) issues a command such as run, step or continue.<br>
> After the command finishes initiating its action, it returns<br>
> up the call stack and goes back into the main command loop<br>
> waiting for user input. Simultaneously, as the inferior<br>
> process starts up, the MonitorChildProcess thread picks up<br>
> the change and posts to the PrivateEvent thread.<br>
> HandePrivateEvent() then calls PushProcessIOHandler() which<br>
> will disable the command IO handler and give the inferior<br>
> control of the TTY. To observe this on OSX, put a<br>
><br>
> usleep(100);<br>
><br>
> immediately prior the PushProcessIOHandler() in<br>
> HandlePrivateEvent.<br>
><br>
><br>
> My proposed solution is that after a 'run', 'step', or<br>
> 'continue' command, insert a synchronization point and wait<br>
> until HandlePrivateEvent knows the inferior process is<br>
> running and has pushed the IO handler. One context switch<br>
> (<100us) is usually all the time it takes on my machine. As<br>
> an additional safety, I have a timeout (currently 1ms) so it<br>
> will never hang the main thread.<br>
><br>
> Any thoughts, or suggestions would be appreciated.<br>
><br>
> Regards,<br>
> Shawn.<br>
><br>
><br>
> To report this email as spam click here<br>
> <<a href="https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==" target="_blank">https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==</a>>.<br>
><br>
><br>
><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a>><br>
><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
><br>
><br>
><br>
><br>
> Member of the CSR plc group of companies. CSR plc registered in<br>
> England and Wales, registered number 4187346, registered office<br>
> Churchill House, Cambridge Business Park, Cowley Road, Cambridge,<br>
> CB4 0WZ, United Kingdom<br>
> More information can be found at <a href="http://www.csr.com" target="_blank">www.csr.com</a><br>
> <<a href="http://www.csr.com" target="_blank">http://www.csr.com</a>>. Keep up to date with CSR on our technical<br>
> blog, <a href="http://www.csr.com/blog" target="_blank">www.csr.com/blog</a> <<a href="http://www.csr.com/blog" target="_blank">http://www.csr.com/blog</a>>, CSR people<br>
> blog, <a href="http://www.csr.com/people" target="_blank">www.csr.com/people</a> <<a href="http://www.csr.com/people" target="_blank">http://www.csr.com/people</a>>, YouTube,<br>
> <a href="http://www.youtube.com/user/CSRplc" target="_blank">www.youtube.com/user/CSRplc</a> <<a href="http://www.youtube.com/user/CSRplc" target="_blank">http://www.youtube.com/user/CSRplc</a>>,<br>
> Facebook, <a href="http://www.facebook.com/pages/CSR/191038434253534" target="_blank">www.facebook.com/pages/CSR/191038434253534</a><br>
> <<a href="http://www.facebook.com/pages/CSR/191038434253534" target="_blank">http://www.facebook.com/pages/CSR/191038434253534</a>>, or follow us<br>
> on Twitter at <a href="http://www.twitter.com/CSR_plc" target="_blank">www.twitter.com/CSR_plc</a><br>
> <<a href="http://www.twitter.com/CSR_plc" target="_blank">http://www.twitter.com/CSR_plc</a>>.<br>
><br>
> New for 2014, you can now access the wide range of products<br>
> powered by aptX at <a href="http://www.aptx.com" target="_blank">www.aptx.com</a> <<a href="http://www.aptx.com" target="_blank">http://www.aptx.com</a>>.<br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a>><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
><br>
><br>
><br>
><br>
><br>
</div></div>> <sbest_iohandler_race_rev_04.diff>_______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>