[Lldb-commits] [PATCH] Fix process attach and detach for Windows

[Adrian McCarthy]

================
Comment at: source/Plugins/Process/Windows/DebuggerThread.cpp:229
@@ -231,1 +228,3 @@
+        // Force a fresh break so that the detach can happen from the debugger thread.
+        if (!::DebugBreakProcess(GetProcess().GetNativeProcess().GetSystemHandle()))
         {
----------------
labath wrote:
> If I understand correctly, you have let the process run freely a couple of lines back.(?)
> 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?
I have thought of that, but I'm not sure how to solve it.

What happens depends on whether m_pid_to_detach is set before or after the inferior hits another exception.

If m_pid_to_detach is already set, and the inferior hits a breakpoint, we will detach.

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.

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.

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).

================
Comment at: source/Plugins/Process/Windows/ProcessWindows.cpp:1013
@@ -913,1 +1012,2 @@
 }
+
----------------
labath wrote:
> This looks unnecessary.
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).

http://reviews.llvm.org/D10404

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/