All minor other than the WaitForDebugEvent setup. Feel free to ignore me if I'm sounding nonsensical. :)
================ Comment at: source/Plugins/Process/Windows/DebugMonitorMessageResults.h:44 @@ +43,3 @@ + protected: + DebugMonitorMessageResult(const DebugMonitorMessage *message); + ---------------- explicit? ================ Comment at: source/Plugins/Process/Windows/DebugMonitorMessages.h:56 @@ +55,3 @@ + protected: + DebugMonitorMessage(MonitorMessageType message_type); + ---------------- explicit? ================ Comment at: source/Plugins/Process/Windows/DebugProcessLauncher.h:32 @@ +31,3 @@ + public: + DebugProcessLauncher(lldb::ProcessSP process); + virtual HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error); ---------------- explicit, if not removed? ================ Comment at: source/Plugins/Process/Windows/DebugProcessLauncher.h:36 @@ +35,3 @@ + private: + lldb::ProcessSP m_process; +}; ---------------- This looks to be unused. Might call it m_parent_process if it's going to be used eventually. ================ Comment at: source/Plugins/Process/Windows/DebugStatusMonitorThread.cpp:30 @@ +29,3 @@ + m_monitor_event = ::CreateEvent(NULL, FALSE, FALSE, NULL); + ::CreatePipe(&m_monitor_pipe_read, &m_monitor_pipe_write, NULL, 1024); +} ---------------- Maybe check returns here. ================ Comment at: source/Plugins/Process/Windows/DebugStatusMonitorThread.cpp:30 @@ +29,3 @@ + m_monitor_event = ::CreateEvent(NULL, FALSE, FALSE, NULL); + ::CreatePipe(&m_monitor_pipe_read, &m_monitor_pipe_write, NULL, 1024); +} ---------------- scottmg wrote: > Maybe check returns here. Need to ::CloseHandle all these somewhere. ================ Comment at: source/Plugins/Process/Windows/DebugStatusMonitorThread.cpp:148 @@ +147,3 @@ + // See if any new processes are ready for debug monitoring. + DWORD result = WaitForSingleObject(monitor_thread->m_monitor_event, 1000); + if (result != WAIT_OBJECT_0 && result != WAIT_TIMEOUT) ---------------- Does this mean it'll frequently block for 1s in the middle of debugging? Seems unfortunate. There's only one DebugStatusMonitorThread instance, right? I feel like it shouldn't be the thread that does the spawning and WaitForDebugEvent. Instead, it should create subthreads that CreateProcess+WFDE, and send back to this thread, which can then to a WaitForMultipleObjects on events coming from the app, and from the child processes being debugged. ================ Comment at: source/Plugins/Process/Windows/DebugStatusMonitorThread.cpp:162 @@ +161,3 @@ + // Empty out the debug event queue before checking whether new processes are trying to be + // debugged. Don't spend alot of time blocking on this, because we want LLDB to be + // responsive to launching new processes under a debugger. ---------------- "a lot" (http://hyperboleandahalf.blogspot.com/2010/04/alot-is-better-than-you-at-everything.html :p) ================ Comment at: source/Plugins/Process/Windows/DebugStatusMonitorThread.cpp:171 @@ +170,3 @@ + MonitorData *monitor_data = iter->second; + if (debug_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT) + { ---------------- I know you're just getting started here, but might want to add RIP_EVENT to this so that the map is maintained in that case. http://reviews.llvm.org/D6037 _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
