================
Comment at: source/Plugins/Process/Windows/DebuggerThread.cpp:374
@@ -358,1 +373,3 @@
+ if (m_pid_to_detach != 0 && m_active_exception->GetExceptionCode() ==
EXCEPTION_BREAKPOINT) {
+ WINLOG_IFANY(WINDOWS_LOG_EVENT | WINDOWS_LOG_EXCEPTION |
WINDOWS_LOG_PROCESS,
----------------
amccarth wrote:
> labath wrote:
> > This is almost a nit, since it will generally work, but technically, this
> > looks like a data race ( => UDB) on the `m_pid_to_detach` variable: You are
> > accessing the variable here and in DebugDetach() and one of those accesses
> > is write. Is there some synchronization here that I am not aware of which
> > is preventing these accesses to occur simultaneously?
> I see your point. DebugDetach, which sets the pid, is called only from
> ProcessWindows under its lock, and then it triggers a breakpoint exception,
> so in the normal case it does work. But if a breakpoint is hit as we're
> setting the pid, there is indeed a data race. Nice catch.
>
> I suppose I could make m_pid_to_detach a std::atomic.
Yes, I believe an atomic variable would suffice in this case.
================
Comment at: source/Plugins/Process/Windows/ProcessWindows.cpp:474
@@ +473,3 @@
+
+ SetPrivateState(eStateDetached);
+
----------------
amccarth wrote:
> labath wrote:
> > I don't know enough about ProcessWindows, but a thought comes to mind here:
> > You set the process state to be eStateDetached, but that is not exactly
> > true (yet). You have merely requested detachment, which is a process that
> > can take a while (especially if the inferior is in the middle of processing
> > some other breakpoint). It seems likely that this activity (processing a
> > breakpoint hit) will race with whatever happens after DoDetach() returns. I
> > would expect some kind of synchronization here that waits for a signal from
> > the debugger thread that the detachment was successful.
> >
> > As I said, I'm not an expert, but I was wondering if you have considered
> > this scenario.
> I'm pretty new to this as well. No, I hadn't considered the scenario where
> we're already stopped at a breakpoint. I was trying to get TestAttachResume
> to pass, which detaches while the inferior is still running.
>
> I'll try to work through the implications of that and update the patch as
> needed. Thanks!
These things are tricky. Wait until you get around to TestConcurrentEvents. :)
We also need a separate debug thread on linux, and getting these
synchronization things right took a lot of time, and I'm still not convinced we
have everything covered.
http://reviews.llvm.org/D10404
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
