<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 18, 2015 at 5:53 PM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">================<br>
Comment at: source/Plugins/Process/Windows/DebuggerThread.cpp:229<br>
@@ -231,1 +228,3 @@<br>
+ // Force a fresh break so that the detach can happen from the debugger thread.<br>
+ if (!::DebugBreakProcess(GetProcess().GetNativeProcess().GetSystemHandle()))<br>
{<br>
----------------<br>
</span><span class="">amccarth wrote:<br>
> labath wrote:<br>
> > If I understand correctly, you have let the process run freely a couple of lines back.(?)<br>
> > What will happen if the process stops again (another breakpoint, watchpoint, access violation, ..) before you get a chance to stop it here? Can such a thing happen?<br>
> I have thought of that, but I'm not sure how to solve it.<br>
><br>
> What happens depends on whether m_pid_to_detach is set before or after the inferior hits another exception.<br>
><br>
> If m_pid_to_detach is already set, and the inferior hits a breakpoint, we will detach.<br>
><br>
> If it's not yet set, or if the inferior stops for another reason (like an access violation, since watchpoints aren't implemented on Windows yet), then the process will stop, then we'll finish the detach, leaving a non-running process on the system. I managed to make this happen once, at which point I was able to re-attach to the process.<br>
><br>
> I could reduce the risk of the latter situation by setting the m_pid_to_detach before the continue (just as the terminate case does), but I even then, I don't think I can guarantee that the next stop will be from our DebugBreakProcess call.<br>
><br>
> What I'd like to do is create more tests for all these cases, which I assume may involve solving the same types of problems for at least some of the other platforms. But my first goal is to get TestAttachResume and TestBreakAfterAttach working, and this gets us much closer (there are follow-up patches for making those tests cross platform and for eliminating more POSIX dependencies from lldbtest.py).<br>
</span>Yeah, this is tricky. On linux, we are detaching from a stopped state, which makes things easier, since we don't have to worry about the inferior doing anything funny while we are trying to detach (or do any other operation for that matter).<br>
<br>
The safest way for this that I can see is to do the `ContinueAsyncException` call and the `DebugActiveProcessStop` on the same thread.</blockquote><div><br></div><div>That's the hitch. Although it's not documented, DebugActiveProcessStop appears to work only if it's issued from the debugger thread that initially attached with DebugActiveProcess. That's why we set the flag, and then call DebugBreakProcess: It's the only way we came up with to signal the debugger thread to detach. I don't see a way to make ContinueAsyncException happen on that thread.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> So we could have the debug thread listen for two things (if such a thing is possible):<br>
- regular debug events as it is doing now<br>
- other events via some other communication channel: using this channel we can signal it to execute the detach<br>
Since these things will then happen on the same thread, we can be sure that the processing some debug event does not interfere with our detach efforts. Does that make any sense?<br>
<span class=""><br>
>What I'd like to do is create more tests for all these cases, which I assume may involve solving the same types of problems for at least some of the other platforms. But my first goal is to get TestAttachResume and TestBreakAfterAttach working, and this gets us much closer (there are follow-up patches for making those tests cross platform and for eliminating more POSIX dependencies from lldbtest.py).<br>
</span>Yes please, the more tests the better. I'm not saying that the other platforms are perfect. And just because your test exposes a bug in some platform, it doesn't mean you have to fix it.<br>
<br>
If you want to commit this as a working concept now, I'm fine with that, but I think we should find a bullet-proof way to do this.<br></blockquote><div><br></div><div>I agree that I should work on making it better. The fact is that attach was only sort-of working and detach was missing altogether, both of which were blocking progress on the multithreaded tests.</div><div><br></div><div>So I'm going to commit this as a working concept, and I'll open a bug against myself the find a way to make it bullet-proof.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
================<br>
Comment at: source/Plugins/Process/Windows/ProcessWindows.cpp:1013<br>
@@ -913,1 +1012,2 @@<br>
}<br>
+<br>
----------------<br>
</span><span class="">amccarth wrote:<br>
> labath wrote:<br>
> > This looks unnecessary.<br>
> The last line '}' was unterminated, and my editor seems to insist on making sure it ends with a newline (which is a requirement of POSIX).<br>
</span>Ok, nevermind then.<br>
<div class="HOEnZb"><div class="h5"><br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10404&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=MEqT8U_n7oNfuDW5NRbY3ZV384ZquXIYFPWmprwUdKM&m=gx0pVTaU-riEVEwBcMae4RtXG_BIGTrP4ftyUJrqP_o&s=43RgYpdFV8nGYpsbBCSGZs-G-8oYwVO9hs9eZWJFyhw&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10404</a><br>
<br>
EMAIL PREFERENCES<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=MEqT8U_n7oNfuDW5NRbY3ZV384ZquXIYFPWmprwUdKM&m=gx0pVTaU-riEVEwBcMae4RtXG_BIGTrP4ftyUJrqP_o&s=Z0ymdukNvb-yLLN6UqdzpEdbbBqtFm9hJK0PNcESaaE&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>