Ok,
Thanks for this, Greg. I'm not in a position to look at this right now,
but wanted to at least point out some of my observations. Hopefully
someone can pick this up sometime soon.
thanks
Matt
Greg Clayton wrote:
On Aug 21, 2014, at 12:27 AM, Matthew Gardiner <m...@csr.com> wrote:
Hi Greg,
If that is the case, then the main thread (i.e. the one running
Debugger::ExecuteIOHandler) needs to be blocked until the event arrives.
The command line command "process connect" needs to synchronize better. It currently isn't and this
is causing the problem. All we know is that we are executing a "gdb-remote 1234" command which
turns into "process connect connect://localhost:1234" and then the command execution returns
immediately and then the io handler gets the next command as it should. We should add code to commands that
we know might needs some syncing to make sure they don't violate things.
Why?
Because with the existing design once the user has entered their "gdb-remote" command,
and completed the connect call, the main thread goes back to the IOHandlerEditline::Run() loop,
sees that IsActive() is true, prints the next prompt, etc.. When I debugged this I didn't see any
call to Deactivate or SetIsDone, which would have made IsActive return false. (And then the async
DefaultEventHandler awakes and it's output "Process 1 stopped" splats over the prompt).
No other IOHandler gets pushed, so the command interpreter is active and it will just get the next
command as soon as it can. Most commands do something, then complete and then we are ready for
more. There are a few that needs some extra syncing because they cause a launch/attach/connect and
we usually get a response right away. There are others that might be immediate and might be async
"thread step-XXX" for example. The step might complete and it might not. There are others
that are purely async "process continue".
To complicate this, if we attach to a process, then there is no process IO
handler to push and the command line will always be active.
If the code is changed so that the edit line IOHandler's IsActive returns false,
while an asynchronous event is happening, then I think that the main thread would
spin, since the reader_sp->Run() function below:
void
Debugger::ExecuteIOHanders()
{
while (1)
{
IOHandlerSP reader_sp(m_input_reader_stack.Top());
if (!reader_sp)
break;
reader_sp->Activate();
reader_sp->Run();
reader_sp->Deactivate();
would immediately return. That's why I'm thinking the main thread probably
should block until the last issued command has completed.
So this is the job of each command. Most commands complete right way.
It sounds like we might want to introduce a formal synchronization to the IOHandler and
each command would need to change it from the default "no sync required".
We would need two things:
- no sync required (default)
- sync with timeout for commands that usually complete quickly, but if they
don't we need to get back to commands
Out of interest, I did research your "If someone is grabbing the event manually by
hijacking events" point. But when stopped state is detected (i.e. the reply to ?) in
GDBRemote and Broadcaster::PrivateBroadcastEvent is called, there is no hijacking_listener.
Indeed the execution path is that as indicated by the -->
if (hijacking_listener)
{
if (unique && hijacking_listener->PeekAtNextEventForBroadcasterWithType
(this, event_type))
return;
hijacking_listener->AddEvent (event_sp);
}
else
{
collection::iterator pos, end = m_listeners.end();
// Iterate through all listener/mask pairs
for (pos = m_listeners.begin(); pos != end; ++pos)
{
// If the listener's mask matches any bits that we just set, then
// put the new event on its event queue.
if (event_type & pos->second)
{
if (unique && pos->first->PeekAtNextEventForBroadcasterWithType
(this, event_type))
continue;
----> pos->first->AddEvent (event_sp);
So my contention is that in the case of gdb-connect the initial stop state should either
be delivered/printed sychronously in the Process::ConnectRemote (i.e. in the mainthread
context) or that the main thread should block until either the event arrives, or for some
other reason the command last issued by the user is deemed to be "complete".
Agreed. "process connect" should be doing better synchronization.
We also need a better way to deliver async output to the IOHandler stack. Right
the only place that tries to handle this is in the Debugger class where it
handles process events. This is where the tricky code of popping the process
IOHandler lives. What should happen is we should have a function like:
Debugger::AsyncIO (...)
and any async output (from the process' stdout/stderr, or an eStateStopped
event with thread state and frame and reason for stop) should be delivered
through this.
The top IOHandler would then get this delivered to it in a thread safe way and it could
then hide itself (for the command interpreter remove the "(lldb) " prompt and
any command you are typing, display the output, then refresh the prompt and command, and
continue. I believe this will solve all of our issues (using Debugger::AyncIO(...)).
Greg
To report this email as spam click
https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .
Member of the CSR plc group of companies. CSR plc registered in England and
Wales, registered number 4187346, registered office Churchill House, Cambridge
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube,
www.youtube.com/user/CSRplc, Facebook,
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at
www.aptx.com.
_______________________________________________
lldb-dev mailing list
lldb-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
