[Ann] ReadWriteLock

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 <[hidden email]> wrote:

On 05 Jan 2016, at 7:52 , Henrik Johansen <[hidden email]> wrote:

On 05 Jan 2016, at 7:24 , Denis Kudriashov <[hidden email]> wrote:

2016-01-05 18:49 GMT+01:00 Henrik Johansen <[hidden email]>:

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

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 <[hidden email]>:

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 <[hidden email]> wrote:

On 05 Jan 2016, at 7:52 , Henrik Johansen <[hidden email]> wrote:

On 05 Jan 2016, at 7:24 , Denis Kudriashov <[hidden email]> wrote:

2016-01-05 18:49 GMT+01:00 Henrik Johansen <[hidden email]>:

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

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 <[hidden email]>:

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 <[hidden email]>:

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 <[hidden email]> wrote:

On 05 Jan 2016, at 7:52 , Henrik Johansen <[hidden email]> wrote:

On 05 Jan 2016, at 7:24 , Denis Kudriashov <[hidden email]> wrote:

2016-01-05 18:49 GMT+01:00 Henrik Johansen <[hidden email]>:

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

2016-01-07 0:15 GMT+01:00 Ben Coman <[hidden email]>:

I'm not qualified to judge, but is sounds okay.  I have an alternative
proposal to consider that hopefully might remove Semaphore handling
completely from Process>>terminate.

What if we have a wait primitive that instead of returning itself
(i.e. a Semaphore), it always returns true.   Then if the process is
terminated while the primitive is waiting, before it consumes a
signal, the assignment to signalConsumed never occurs.  Then we
have...

  critical: mutuallyExcludedBlock
    signalConsumed := false.
    [
        signalConsumed := self primWaitSucceeded.
        blockValue := mutuallyExcludedBlock value
    ] ensure: [ signalConsumed ifTrue: [self signal] ].
    ^blockValue

It *feels* more atomic.  Someone knowledgeable with the vm would need
to advise whether after the primitive wakes up and consumes a signal,
the assignment into signalConsumed can be guaranteed to occur before
the process gets a chance to terminate, even if the primitive needs to
directly poke the value into signalConsumed.

To me it is better too. But it requires changes in VM primitive which I'm can't provide.

So I will implement my idea. Do you ok with the name #waitIfInterrupted?

Ugh, there's a bug with the Processor activeProcess suspendingList, I only checked in the case where it *should* signal.

Take it as an inspiration :)

Cheers,

Henry

On 08 Jan 2016, at 2:40 , Henrik Johansen <[hidden email]> wrote:

Here's an alternative implementation, which, instead of checking for the new method name, hooks into the existing unwind machinery to execute the waitIfCurtailed: argument.

Which removes the special case from Process >> #terminate, but at the added cost that in-progress unwinds are executed as if the (suspended) terminating thread they started executing on, is still the active process. (Which, arguably, should be done anyways)

Cheers,

Henry

<SignalUsingUnwindMachinery.cs>

"This runs on UI thread, pri 40"

waiter := Semaphore new.

waiter2 := Semaphore new.

lock := Semaphore forMutualExclusion.

proc := [lock critical2: [ Transcript crShow: '1 start'.

waiter wait. 

Transcript crShow: '1 done'.]] newProcess.

proc priority: 70.

proc resume.

proc2 := [lock critical2: [ 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') 1

On 07 Jan 2016, at 11:09 , Denis Kudriashov <[hidden email]> wrote:

Ok. slice 17340 in inbox. It is only touches current #critical: implementation. It is not about deprecation wrong semaphore usage

2016-01-07 9:55 GMT+01:00 Denis Kudriashov <[hidden email]>:

2016-01-07 0:15 GMT+01:00 Ben Coman <[hidden email]>:

I'm not qualified to judge, but is sounds okay.  I have an alternative
proposal to consider that hopefully might remove Semaphore handling
completely from Process>>terminate.

What if we have a wait primitive that instead of returning itself
(i.e. a Semaphore), it always returns true.   Then if the process is
terminated while the primitive is waiting, before it consumes a
signal, the assignment to signalConsumed never occurs.  Then we
have...

  critical: mutuallyExcludedBlock
    signalConsumed := false.
    [
        signalConsumed := self primWaitSucceeded.
        blockValue := mutuallyExcludedBlock value
    ] ensure: [ signalConsumed ifTrue: [self signal] ].
    ^blockValue

It *feels* more atomic.  Someone knowledgeable with the vm would need
to advise whether after the primitive wakes up and consumes a signal,
the assignment into signalConsumed can be guaranteed to occur before
the process gets a chance to terminate, even if the primitive needs to
directly poke the value into signalConsumed.

To me it is better too. But it requires changes in VM primitive which I'm can't provide.

So I will implement my idea. Do you ok with the name #waitIfInterrupted?