On 05 Jan 2016, at 5:25 , Denis Kudriashov <dionisiydk@gmail.com> wrote:
I found problem. Look at Process>>terminate. There is special place:
"Figure out if we are terminating the process while waiting in Semaphore>>critical: In this case, pop the suspendedContext so that we leave the ensure: block inside Semaphore>>critical: without signaling the semaphore." (oldList class == Semaphore and: [ self halt. suspendedContext method == (Semaphore compiledMethodAt: #critical:) ]) ifTrue: [ suspendedContext := suspendedContext home ].
Really crazy. Comment inside #critical: method said that we should signal in any case. But #terminate method said that we should not allow signalling in that case. Why it is not implemented inside #critical: method by #ifCurtailed: logic?
ifCurtailed: only unwinds if an error/termination occured, so that would mean not signalling the semaphore when everything goes as planned, and we leave the critical section... There are only two cases: - Have I been unblocked by the semaphore? (IE, have I consumed a signal?) Then, signal when exiting section (whether through normal execution, termination, or an unhandled error). - Has it not? Then, do not signal. Here, there are two subcases - I have entered the section that will unwind in the ensure: block, but am not yet waiting for Semaphore. (handled by caught variable) - I have started waiting for semaphore, but not consumed a signal. (handled by #terminate). The last bit is potentially really tricky, if the callback code in suspend is actually used, and is not performed atomically. (Strangely, a haltOnce in the suspend fallback code *did* trigger for me, despite the comment that it is purely for old VM's) If the terminating process is interrupted on the remove:ifAbsent: call (which could/should be removeLink:ifAbsent:, btw) by a thread signalling its semaphore, and the semaphore selects the process being terminated as the one to resume, but terminating thread is resumed before terminated (which is no longer *really* suspended), the fallback code will happily return the (now no longer really suspended) semaphore it recorded before removeLink:ifAbsent: call. The terminating process then checks that it is indeed a semaphore, and that the terminating threads suspendedContext is still the critical call, and promptly decides to not signal the semaphore as part of termination. The net result of which, would be an entered critical section without a corresponding exit, and a hung semaphore until someone feeds it a few extra signals, if we're talking input semaphore, say, by wiggling the mouse a bit or something... Sound familiar to anyone? A possible fix would be changing: oldList remove: self ifAbsent: [] to oldList removeLink: self ifAbsent:[oldList := nil] In other words. if someone (ie, the waiting list ) has removed a thread from the old waiting list before the thread was able to do it, don't just proceed as if nothing has happened, but consider the terminated thread as no longer having on the old waiting list at the time of suspension. Then, terminate will find a nil oldList (whose class is not Semaphore), and the ensure: block will execute as the semaphore expects. Combined with the suggesting in preceeding mail, that would mean not doing the checks in terminate, but #cull:'ing the oldList into ensure contexts during unwind. Cheers, Henry Timeline: [Terminating Thread] Wanting to terminate a victim thread, it tries to suspend it. [Terminating Thread] Suspend call records the oldList (semaphore) victim is currently blocked on , then is interrupted at the remove:ifAbsent: call. [Signalling Tread] Signals the semaphore [Signalling Tread] Semaphore decides to resume the victim thread. Primitive removes it from list, and readies it for resumption. [Signalling Tread yields, expecting a signal from victim thread when it is done] [Terminating Thread resumed] [Terminating Tread] Checks oldList and suspendedContext, find them both satisfying its check for whether the victim was waiting for a semaphore, and pops the ensure block