On 14 Jul 2008, at 7:49 PM, Adam Maxwell wrote: > On 2008-07-14 10:10:15 -0700, Christiaan Hofman <[EMAIL PROTECTED]> > said: > >> >> 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. > > Blocking with a timeout isn't the same as polling with a timer. Using > a runloop with a timeout is recommended here: > > http://developer.apple.com/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/chapter_6_section_4.html > > notably > > on the section "Exiting the Run Loop." > >>> 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. > > I was suggesting that the flag be set in the server thread; changing > the order wouldn't work...(I see you noticed that below). > >> 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. > > It won't happen until the end of the calculation loop anyway unless > the > calculation loop runs the runloop and checks that flag, though, right? >
It may. The shouldKeepRunning flag is checked in various places, in particular in while-loops, so the long loops are terminated when it is reset. That's very important. For example the file content search index can take an hour, so it is important that it knows when it's no longer needed. Christiaan >> 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. > > Yes, deallocating at the end of runDOServerForPorts: ought to work, > and > is probably cleaner. I'd have it call -cleanup for backwards > compatibility, though, since numerous subclasses override it. I > wouldn't guarantee that there are no dependencies on the order of > -cleanup vs. other teardown, either. > > -- > adam ------------------------------------------------------------------------- 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
