public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* event-loop thread changing
@ 2007-04-18  3:30 Andrew Cagney
  2007-04-19 13:43 ` Mark Wielaard
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cagney @ 2007-04-18  3:30 UTC (permalink / raw)
  To: frysk

Hi,

Linux's ptrace that all ptrace operations originate from the same thread 
- tid1.attatch, tid2.peek is not allowed.  Frysk currently works round 
this limitation by having a dedicated ptrace server thread.

I'm now trying to eliminate that server thread and its overhead by, 
instead, having the event-loop thread handle ptrace calls locally.  In 
trying to implement the change I've found that a number of test-cases 
are switching the thread running the event-loop mid-stream.  Typically 
this happens with something like:

    eventLoop.requestSchedule();
    eventLoop.run();
    ...;
    eventLoop.start();

I've added checks to EventLoop to detect this - right now they complain 
but not too loudly - going forward it will trigger a panic.  Even 
without the ptrace issue, changing threads is bad.

The best approach here is to run the event-loop from the main test 
thread, and then run additional parallel operations from separate 
short-term threads.  See TestBreakpoints for an example.  If nothing 
else, it ensures that any event-loop panic occures within the test 
thread allowing junit to correctly clean up.

Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: event-loop thread changing
  2007-04-18  3:30 event-loop thread changing Andrew Cagney
@ 2007-04-19 13:43 ` Mark Wielaard
  2007-04-19 14:32   ` Andrew Cagney
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Wielaard @ 2007-04-19 13:43 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: frysk

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

Hi Andrew,

On Tue, 2007-04-17 at 13:19 -0400, Andrew Cagney wrote:
> I'm now trying to eliminate that server thread and its overhead by, 
> instead, having the event-loop thread handle ptrace calls locally.  In 
> trying to implement the change I've found that a number of test-cases 
> are switching the thread running the event-loop mid-stream.  Typically 
> this happens with something like:
> 
>     eventLoop.requestSchedule();
>     eventLoop.run();
>     ...;
>     eventLoop.start();
> 
> I've added checks to EventLoop to detect this - right now they complain 
> but not too loudly - going forward it will trigger a panic.  Even 
> without the ptrace issue, changing threads is bad.

While you are cleaning this up you might want to make EventLoop just
implement Runnable and not extend Thread. That way people don't get to
accidentally use things like eventLoop.start() or eventLoop.join()
thinking the can just treat it as an Thread. Seems only the
EventLoopTestBed, fstack and the Coredump/StackTraceActions depend on it
being a separate Thread object. And the last 3 seem to really just want
to know when the eventloop has terminated, not really join() with it as
a Thread.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: event-loop thread changing
  2007-04-19 13:43 ` Mark Wielaard
@ 2007-04-19 14:32   ` Andrew Cagney
  2007-04-19 20:10     ` Mark Wielaard
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cagney @ 2007-04-19 14:32 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: frysk

Mark,

I'm not sure I follow.  The calls to .start() I've seen were all 
intentional:

- do some stop/start actions with the event-loop in the current thread
- [later] create a separate event-loop thread

It is just that with the event-loop handling ptrace, the operation isn't 
allowed.  Instead, either:

- create the event-loop thread calling eventLoop.start() before anything 
else
xor
- run the event-loop from the main thread always

Looking at the second case in more detail, the code was originally more 
like:
    class EventLoopRunner
      extends Thread
    {
       run() { eventLoop.run() }
       waitforloopexit () { convoluted logic here }
    }
    new EventLoopRunner();
but I reduced it to eventLoop.start.

This actually leads us to a real bug, both the above, and:
    new Thread(eventLoop).start()
are racy.  This returns, allowing event-loop calls to be made, before 
the event-loop is ready to handle such requests.  The overriden 
eventLoop.start() addresses this problem.

If code is trying to do a .join() then there is a problem.

The one issue I do know is code wanting to get a notification when the 
event-loop panics - both the HPD and FryskGui do this, we need look at 
extending event-loop to support this.

Andrew

Mark Wielaard wrote:
> Hi Andrew,
>
> On Tue, 2007-04-17 at 13:19 -0400, Andrew Cagney wrote:
>   
>> I'm now trying to eliminate that server thread and its overhead by, 
>> instead, having the event-loop thread handle ptrace calls locally.  In 
>> trying to implement the change I've found that a number of test-cases 
>> are switching the thread running the event-loop mid-stream.  Typically 
>> this happens with something like:
>>
>>     eventLoop.requestSchedule();
>>     eventLoop.run();
>>     ...;
>>     eventLoop.start();
>>
>> I've added checks to EventLoop to detect this - right now they complain 
>> but not too loudly - going forward it will trigger a panic.  Even 
>> without the ptrace issue, changing threads is bad.
>>     
>
> While you are cleaning this up you might want to make EventLoop just
> implement Runnable and not extend Thread. That way people don't get to
> accidentally use things like eventLoop.start() or eventLoop.join()
> thinking the can just treat it as an Thread. Seems only the
> EventLoopTestBed, fstack and the Coredump/StackTraceActions depend on it
> being a separate Thread object. And the last 3 seem to really just want
> to know when the eventloop has terminated, not really join() with it as
> a Thread.
>
> Cheers,
>
> Mark
>   

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: event-loop thread changing
  2007-04-19 14:32   ` Andrew Cagney
@ 2007-04-19 20:10     ` Mark Wielaard
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2007-04-19 20:10 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: frysk

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

Hi Andrew,

On Thu, 2007-04-19 at 10:17 -0400, Andrew Cagney wrote:
> I'm not sure I follow.  The calls to .start() I've seen were all 
> intentional:
> 
> - do some stop/start actions with the event-loop in the current thread
> - [later] create a separate event-loop thread
> 
> It is just that with the event-loop handling ptrace, the operation isn't 
> allowed.

Right. And that is why I suggest getting rid of one of the options
(actually it would probably be better to also not make it implement
Runnable if you expect people to not use run() either). Exposing the
EventLoop as a Thread make people believe they can manipulate it as
such, which is clearly not what we intend. So just having one entry
point for starting/stopping (and in the case where people use
Thread.join() a eventloop died notification) makes it so people don't
accidentally do that. And by changing the class type you automatically
catch the bad users. The actual Thread that the event loop uses can then
just be hidden from the user and you could even turn the EventLoop into
a singleton since that is what you seem to want really that once it is
initialized it doesn't "change threads" later on.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-04-19 14:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-18  3:30 event-loop thread changing Andrew Cagney
2007-04-19 13:43 ` Mark Wielaard
2007-04-19 14:32   ` Andrew Cagney
2007-04-19 20:10     ` Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).