[lldb-dev] Stop/Resume process, interrupting I/O

[Mario Zechner]

Yes, that all makes sense to me. It will somewhat complicate the JDWP
thread logic, but i guess it will ultimately safe me quite a bit of pain.

Thanks!

On Mon, Oct 27, 2014 at 9:23 PM, Greg Clayton <gclayton at apple.com> wrote:

>
> > On Oct 27, 2014, at 12:42 PM, Mario Zechner <badlogicgames at gmail.com>
> wrote:
> >
> > The piggy backing of events is fine from my point of view as long as i
> can check the interrupted flag (for which i'll try to compose a patch). My
> issues stem more from the fact that i have to concurrently operate on the
> process (event loop thread, JDWP thread). The "hook" breakpoints are
> actually handled on the event loop thread, including the auto-resume, which
> is a requirement for our JDWP implementation to work correctly. The JDWP
> client shouldn't know that the process hit a "hook" breakpoint.
>
> I would venture to say that trying to handle events on different threads
> and doing a handoff is a bad idea (race conditions, etc) and you should
> handle process events on one thread.
> >
> > My problem is really this: the JDWP thread stops the event loop thread.
> Next it stops the process via SBProcess::Stop (which calls Process::Halt).
> Sadly, Process::Halt doesn't set the state on the process. I have to poll
> events on the JDWP thread for that to happen, on the JDWP thread, until i
> get a stopped (or exited/crashed/...) event. I then have to re-broadcast
> those events so the event loop thread can later process them once i resume
> it. I'm very worried about re-broadcasting events as i don't know what
> havoc that will wreck in the LLDB internal state.
>
> This what "sync" mode is for:
>
> debugger.SetAsync(false)
>
> When in this mode, any process events that resume the process will wait
> until the process stops before returning:
>
> process.Continue()
>
> This continue won't return until the process actually stops. It will
> consume all running/stopped events.
>
>  But as you already have a thread that is handling events, I still say
> this is a bad idea. What we do in Xcode is have a thread that is waiting
> not only for process events, but also for other events like common things
> to do like backtraces, get frame variables, etc). So you
>
> >
> > Ideally, Process::Halt (and hence SBProcess::Stop()) would set things up
> in a way where i don't have to poll events on the JDWP thread. It should
> also set the process state accordingly. I also noticed strange timing bugs
> when calling SBProcess::Stop() while a previous SBProcess::Continue() is
> still being propagated to the inferior. That's actually what causes the
> eStopReasonInvalid events i talked about in another thread.
> >
> > As always, really appreciate that you take the time to read and answer
> my walls of text.
>
> So I would say the safest thing to do is to make your event handling
> thread always handle all events. If you need the JDWP thread to do
> something, it sends a event over to the event thread and has it be handled.
> You can make your own broadcaster and then listen to it as well on the
> thread that is listening for process events. This way you don't mix and
> match. The event thread can also use that same broadcaster to send and
> event back to the JDWP thread.
>
> Something like:
>
> SBBroadcaster g_broadcaster("JDWP");
>
> enum JDWPEvents {
>     eEventThreadCommand    = 1u << 0,
>     eJDWPThreadResponse    = 1u << 1
> };
>
> int event_thread(...)
> {
>     SBDebugger debugger = lldb::SBDebugger::Create();
>     debugger.SetAsync (true);
>     SBError error;
>     uint32_t event_timeout_seconds = UINT32_MAX; // Infinite wait
>     SBTarget target = debugger.CreateTarget (...)
>     if (target.IsValid())
>     {
>         SBListener listener("event_thread.listener");
>
>         SBProcess process = target.Launch (listener,
>                                            NULL,    // char const **argv,
>                                            NULL,    // char const **envp,
>                                            NULL,    // const char
> *stdin_path,
>                                            NULL,    // const char
> *stdout_path,
>                                            NULL,    // const char
> *stderr_path,
>                                            NULL,    // const char
> *working_directory,
>                                            0,       // uint32_t
> launch_flags
>                                            false,   // bool stop_at_entry,
>                                            error);
>
>         listener.StartListeningForEvents (g_broadcaster,
> eEventThreadCommand);
>
>         if (error.Success() &&
>             process.IsValid() &&
>             process.GetProcessID() != LLDB_INVALID_PROCESS_ID)
>         {
>             SBEvent event;
>
>             if (listener.WaitForEvent (event_timeout_seconds, event))
>             {
>                 if (event.BroadcasterMatchesRef(g_broadcaster))
>                 {
>                     // eEventThreadCommand
>                     // Do the work needed by the JDWP thread, and then
> send a response to let it know it is done...
>
> g_broadcaster.BroadcastEventByType(eJDWPThreadResponse);
>                 }
>                 else
>                 {
>                     // Process event...
>                 }
>             }
>         }
>     }
> }
>
> int jdwp_thread(...)
> {
>     SBListener listener("jdwp_thread.listener");
>     listener.StartListeningForEvents (g_broadcaster, eEventThreadCommand);
>     if (listener.WaitForEvent (event_timeout_seconds, event))
>     {
>     }
> }
>
>
> One thing we need to add to:
>
>     SBProcess
>     SBTarget::Launch (SBLaunchInfo &launch_info, SBError& error);
>
> Is the ability to set the SBListener in the SBLaunchInfo so we can specify
> an alternate listener. Do you get where I am going with this? The key is to
> listen for events and do all process control from one thread to ensure
> there are no races.
>
> Greg
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20141027/936e5794/attachment.html>