[Lldb-commits] [PATCH] Fix handling of hijacked events in synchronous mode

[jingham at apple.com]

> On May 14, 2015, at 10:51 AM, Ilia K <ki.stfu at gmail.com> wrote:
> 
> In http://reviews.llvm.org/D9371#172280, @jingham wrote:
> 
>> I don't like the idea of SBProcess::CreateProcessEventStateChanged.  That makes it sound like it is okay to fabricate StateChanged events of any sort and send them to the world, as though that wouldn't in the general case cause all sorts of havoc.
> 
> 
> I dislike it too.
> 
>> Do you need a real process event in order to handle this?  I don't know how the MI is hooked up, but it seems GenerateStopEventIfNeeded could just tell the MI that the command you just interpreted resulted in a stop and that would trigger emitting a *stopped event.
> 
> 
> Unfortunately not. The *stopped notification is printed by event-thread (that handles events :)), and it sleeps until a new event come. After that, it calls HandleEvent(event) then HandleEventSBProcess(event) -> HandleProcessEventBroadcastBitStateChanged(event) -> HandleProcessEventStateStopped(event).
> I can't just call HandleProcessEventStateStopped without the instance of SBEvent.
> Another reason is that the ProcessEventData contains the m_interrupted flag which is set when target was interrupted, and I can't predict it. 

This is not going to be handled by the your "CreateProcessEventStateChanged" since that has no way of knowing whether the event you are emulating was interrupted.  You also need to know whether the event was a stop & restart, which you don't know.

> 
>> If that's not possible, something more narrow like RebroadcastStopEvent, that would check to make sure we really are stopped, and then generate the stop event otherwise it would return an empty event, just so that it's clear that this has to be tied to the real state of the process would be a little better.
> 
> 
> Do you mean to add SBProcess::RebroadcastStopEvent()? I think it's also a wrong way. Public API shouldn't contain something like that "hack".

I like it more than "generate random stop event" since it says exactly what you are doing, you are choosing to rebroadcast the last stop.  

An even more explicit way of doing this (though it would require a little side-bookkeeping that we don't currently do) would be SBProcess::GetStopEventForStopID(uint32_t stop_id).  That might actually be more generally useful, though of course if it wasn't the current stop id, you wouldn't currently be able to reconstruct thread states and the like.  But one of our long-term goals for lldb is to be able to store and replay "historical" stop events, with some API to say what you wanted to gather at these historical points.  So it would fit in with that direction.

> 
> Can you explain me, why you don't want to broadcast a stop event in sync mode? I agree that it's not what the sync mode means, but if nobody listening broadcaster, then this event will be dropped without any effects, or it will make sense if someone want to receive these events (like lldb-mi).
> 

What if there is a Listener fetching events, but then for some reason somebody beneath that Listener switches to sync mode, does some stuff that stops & starts the target, and then switches to async mode on the way out.  The agent which is fetching events is going to see some stopped event with no corresponding running event.  Depending on what the code using async does it could even see a bunch of these, some while the sync code is still working.  How is it going to understand that?

Jim


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