I realize that current approach to handle situation in the #terminate method is the only way.

But we need to generalize it somehow and make more explicit. It should be visible inside method which required such logic (like current Semaphore>>critical:).

You suggest something like culling unwind context to ensure method. But it looks like too radical solution. And it is not easy to implement.

Cant we implement Semaphore>>waitIfInterrupted: interruptingBlock? And we put in terminating logic only this knowledge. Terminate method should detect this method and call interruptingBlock . So clients can set simple flag in such cases that lock was not acquired. For example:

critical: aBlock

[signalRequired := true.
semaphore waitIfInterrupted: [signalRequired := false].
^aBlock value] ensure: [signalRequired ifTrue: [semaphore signal]]

What you think?

2016-01-06 14:48 GMT+01:00 Denis Kudriashov <dionisiydk@gmail.com>:

I publish slice with MutexTest. So future changes around all this will be verified by CI

2016-01-06 14:32 GMT+01:00 Denis Kudriashov <dionisiydk@gmail.com>:
Ok. I wrote MutextTests which show problems. See in attachment.��
Attached Mutex.st contains fix which suggest Ben for his scenario. But it broke other scenario.��

Mutex>>critical: aBlock
"Evaluate aBlock protected by the receiver."
| activeProcess signalRequired blockValue |
activeProcess := Processor activeProcess.
activeProcess == owner ifTrue:[^aBlock value].
signalRequired := false.
[
signalRequired := true.
semaphore wait.
owner:= activeProcess.
blockValue := aBlock value
] ensure: [signalRequired & (owner == activeProcess) ifTrue: [owner := nil. semaphore signal]].
^blockValue

My idea of ifCurtailed: usage not helps too. But if you change MutexTest to use Semaphore instead of Mutex then all tests will be green.

2016-01-06 13:38 GMT+01:00 Henrik Johansen <henrik.s.johansen@veloxit.no>:
If you run it, and are confused '2 start' never shows up in transcript, I intended the start messages to be outside critical blocks, sorry for old copypasta.

Cheers,
Henry

On 06 Jan 2016, at 1:29 , Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

On 05 Jan 2016, at 7:52 , Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

On 05 Jan 2016, at 7:24 , Denis Kudriashov <dionisiydk@gmail.com> wrote:

2016-01-05 18:49 GMT+01:00 Henrik Johansen��<henrik.s.johansen@veloxit.no>:
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...

I not suppose to replace #ensure: with #ifCurtailed:. I think about:

signalRequired := false.
[
[signalRequired := true.
self wait] ifCurtailed: [signalRequired := false].
blockValue := mutuallyExcludedBlock value
] ensure: [signalRequired ifTrue: [self signal]].
^blockValue

Ah, that makes more sense, yes, much simpler than what I wrote!
AFAICT, it should work too ;)

After thinking a bit more, it suffers from a similar edge case where the semaphore has been signaled, but process is terminated before it's had a chance to run, after changing to ifCurtailed and cleaning #terminate:

"This runs on UI thread, pri 40"
waiter := Semaphore new.
waiter2 := Semaphore new.
lock := Semaphore forMutualExclusion.

proc := [lock critical: [ Transcript crShow: '1 start'.
waiter wait.��
Transcript crShow: '1 done'.]] newProcess.
proc priority: 70.
proc resume.

proc2 := [lock critical: [ Transcript crShow: '2 start'.
waiter2 wait.
��Transcript crShow: '2 done'. ]] newProcess.
proc2 priority: 15.
proc2 resume.

"Yield for proc2 so it gets to waiting on lock"
1 second wait.
"Signal waiter and yield, causing proc1 to run, and release the lock. lock will then remove proc2 from waiting list"
waiter signal.
Processor yield.
"We have a higher pri than proc2, and will resume execution here before proc2 has chance to run"
proc2 terminate.
"proc2 has been curtailed before running, thus ifCurtailed: guard is still on stack and will be run as part of termination."
(lock instVarNamed: 'excessSignals') 0 "Should be 1"

Cheers,
Henry