On 14 Jul 2008, at 6:31 PM, Adam Maxwell wrote:
> On 2008-07-13 08:28:24 -0700, Christiaan Hofman <[EMAIL PROTECTED]>
> said:
>
>> Adam, I hope you still see this. I am a bit uncertain whether
>> BDSKAsynchronousDOServer is stopped properly. Especially if the
>> server
>> thread runloop is stopped, and whether -cleanup is guaranteed to be
>> executed. I could imagine the following errors to occur:
>>
>> 1. send -cleanup
>> 2. set shouldKeepRunning to 0
>> 3. server thread runloop finishes some long calculation, no new
>> runloop run as shouldKeepRunning == 0
>>
>> Result:
>> -cleanup is never run, because there is no runloop to pick it up
>
> Sounds plausible. I'm not sure of a way around this, since turning
> "&&
> didRun" into "|| didRun" means that the thread could keep running
> forever unless all the inputs are removed, so it would no longer be a
> guaranteed stop mechanism.
>
>> 1. send -cleanup
>> 2. serverThread executes -cleanup immediately
>> 3. server thread starts new runloop run
>> 4. set shouldKeepRunning to 0
>>
>> Result:
>> the runloop on the server thread never finishes, as it gets no input
>> anymore
>
> This also seems possible, although invalidating the connection may
> tickle the runloop. Having only thought about this briefly, a safe
> workaround might be to do this in cleanup:
>
> OSAtomicCompareAndSwap32Barrier(1, 0, (int32_t
> *)&serverFlags->shouldKeepRunning);
> [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode
> beforeDate:[NSDate distantPast]];
>
> which would ensure the runloop exits. Alternately, running it with a
> short timeout in runDOServerForPorts: instead of +[NSDate
> distantFuture] would work. IIRC I use the finite timeout for a
> similar
> problem in FVConcreteOperationQueue.
>
That's usually not recommended, because it amounts to polling.
Moreover you would need to wait at least the timeout before it's
finished.
> In fact, setting shouldKeepRunning = 0 in -cleanup would solve the
> first problem, also (I think), providing the message doesn't get
> dropped. There may be some subtle reason it's set in the main thread,
> but there's no comment to that effect.
That's what I was thinking of, just change the order of the two lines.
The main reason it's set in the main thread is that a running (long)
task may decide to stop executing, e.g. in a long while-loop. If
shouldKeepRunning is set to 0 on the server thread it won't happen
till the current calculation has finished.
>
>> Am I right that this could happen? As you may know I am not very well
>> at run loops and threads.
>
> That makes two of us...I think you made a good catch. There are a lot
> of tricky issues to get right, especially with teardown of the
> connection and the async message sends. I'm less enamored with this
> class now that Apple no longer recommends using DO for interthread
> communication. NSOperationQueue and OFMessageQueue require less
> tweaking, although the latter is only good for void messages.
>
> --
> adam
If the order of the two lines is reversed, I think the only problem
can be that cleanup may not be called. So why not do the real cleanup
at the end of -runDOServerForPorts:? I think the only real reason not
to do that was that the runloop needs to be stopped, but it could just
as well be tickled by a further empty (and therefore irrelevant)
method. Then, if that method was not send because the runloop has
already finished, it's not a problem.
So could the following work?
- (void)stopDOServer {
OSAtomicCompareAndSwap32Barrier(1, 0, (int32_t *)&serverFlags-
>shouldKeepRunning);
// really only send -stop to tickle the runloop so it can finish;
if it comes too late there's no problem
[serverOnServerThread stop];
// rest of cleanup on main thread
}
- (oneway void)stop {
// this is really not necessary, as it's already been done on the
main thread
OSAtomicCompareAndSwap32Barrier(1, 0, (int32_t *)&serverFlags-
>shouldKeepRunning);
}
- (void)serverDidStop {}
- (void)runDOServerForPorts:(NSArray *)ports;
{
// this is unchanged, setup and run the runloop
@finally {
// do the server cleanup here
[localThreadConnection setRootObject:nil];
// this frees up the CFMachPorts created in -init
[[localThreadConnection receivePort] invalidate];
[[localThreadConnection sendPort] invalidate];
[localThreadConnection invalidate];
[localThreadConnection release];
localThreadConnection = nil;
[serverOnMainThread release];
serverOnMainThread = nil;
serverThread = nil;
[self serverDidStop];
[pool release];
}
}
Christiaan
-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Bibdesk-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bibdesk-develop
