[Ann] ReadWriteLock

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
54 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen

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 ;)

+1 from me for fixing Mutex to work under termination, change current Semaphore >> critical: users, and deprecating Semaphore >> #critical: ,after changing it to use ifCurtailed: as you suggested and removing the special casing from Process >> #terminate.

Cheers,
Henry

signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

stepharo
In reply to this post by Denis Kudriashov
This code looks quite bad to me.


Le 5/1/16 17:25, Denis Kudriashov a écrit :
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? 


2016-01-05 17:10 GMT+01:00 Denis Kudriashov <[hidden email]>:

2016-01-05 16:54 GMT+01:00 Denis Kudriashov <[hidden email]>:
2016-01-05 16:06 GMT+01:00 Ben Coman <[hidden email]>:
This is really strange!  Why does the copied class Semaphore2 behave
differently to the original class Semaphore? This was in build 50510.

It is so frustrating.
I add tests for your cases which is red now. And one for Semaphore (inside ReadWriteLockTests) which is green

It is really crazy. I just try two experiments:
1) I move semaphore critical: logic to Mutex. Our test with mutex become red.
2) I copy Semaphore>>critical: to #critical2:. And with it Semaphore test become red. 

So somebody definitely know about #critical: selector and it receiver


Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

stepharo
In reply to this post by Ben Coman
Ben

would you not be interested to write a little chapter on concurrency
because we would all benefit from it.
I can help but I'm pretty bad with concurrency.
I'm sure that doing that we will find a lot of things to improve in Pharo :)

Stef

Le 5/1/16 18:15, Ben Coman a écrit :

> On Tue, Jan 5, 2016 at 11:59 PM, Denis Kudriashov <[hidden email]> wrote:
>> 2016-01-05 16:06 GMT+01:00 Ben Coman <[hidden email]>:
>>> btw I have this growing suspicion that we should eliminate
>>> Semaphore>>critical: and Semaphore>>forMutualExclusion.   Semaphores
>>> on their own should *not* provide a facility for to protect critical
>>> regions since they suffer from accidental release [1] due to
>>> *lack*of*ownership*.
>>
>> I have same feeling when analyse problem of critical: implementation. But I
>> not think that Semaphore>>forMutualExclusion is bad.
> But Semaphore>>forMutualExclusion *encourages* people to believe
> semaphores can be used *on*their*own* for mutual exclusion, when they
> *shouldn't*.  You *must* add ownership.  If the mutual exclusion
> object doesn’t have ownership then, irrelevant of what it is called,
> it is not a mutex!!! [1].
>
> "Strictly speaking [2]...
>    -- A mutex is *locking* mechanism used to synchronize access to a
> resource. Only one task (can be a thread or process based on OS
> abstraction) can acquire the mutex. It means there is *ownership*
> associated with mutex, and only the owner can release the lock
> (mutex).
>    -- A semaphore is *signaling* mechanism (“I am done, you can carry
> on” kind of signal). For example, if you are listening songs (assume
> it as one task) on your mobile and at the same time your friend calls
> you, an interrupt is triggered upon which an interrupt service routine
> (ISR) signals the call processing task to wakeup
>
>
>> It is common case that semaphore "free" by default.
> With an implementation like..
>      Semaphore>>forMutualExclusion
>          ^self new signal
>
> we don't gain a lot, and its a misleading convenience method since its
> doesn't provide the proper facility for mutual exclusion (and I do
> conflate mutual exclusion == mutex).   Indeed, "Semaphore
> forMutualExclusion + roll your ownership management + problems since
> you weren't aware you needed to roll your own ownership management" is
> not more convenient than "Mutex new"
>
> If you want to keep a convenience method for a pre signalled
> Semaphore, then we could provide something like  "Semaphore
> newSignalled"
>
>> And this name is explicitly said about this.
> Actually no.  It explicitly says it provides mutual exclusion - which
> without ownership, it does not. It only implicitly provides a
> pre-signalled Semaphore - you have to *know* of look at the source.
>
> [1]  https://blog.feabhas.com/2009/09/mutex-vs-semaphores-%E2%80%93-part-2-the-mutex/
> [2] http://www.geeksforgeeks.org/mutex-vs-semaphore/
>
> cheers -ben
>
>


Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Ben Coman
okay. I'll have a go.
cheers -ben

On Wed, Jan 6, 2016 at 3:28 AM, stepharo <[hidden email]> wrote:

> Ben
>
> would you not be interested to write a little chapter on concurrency because
> we would all benefit from it.
> I can help but I'm pretty bad with concurrency.
> I'm sure that doing that we will find a lot of things to improve in Pharo :)
>
> Stef
>
> Le 5/1/16 18:15, Ben Coman a écrit :
>
>> On Tue, Jan 5, 2016 at 11:59 PM, Denis Kudriashov <[hidden email]>
>> wrote:
>>>
>>> 2016-01-05 16:06 GMT+01:00 Ben Coman <[hidden email]>:
>>>>
>>>> btw I have this growing suspicion that we should eliminate
>>>> Semaphore>>critical: and Semaphore>>forMutualExclusion.   Semaphores
>>>> on their own should *not* provide a facility for to protect critical
>>>> regions since they suffer from accidental release [1] due to
>>>> *lack*of*ownership*.
>>>
>>>
>>> I have same feeling when analyse problem of critical: implementation. But
>>> I
>>> not think that Semaphore>>forMutualExclusion is bad.
>>
>> But Semaphore>>forMutualExclusion *encourages* people to believe
>> semaphores can be used *on*their*own* for mutual exclusion, when they
>> *shouldn't*.  You *must* add ownership.  If the mutual exclusion
>> object doesn’t have ownership then, irrelevant of what it is called,
>> it is not a mutex!!! [1].
>>
>> "Strictly speaking [2]...
>>    -- A mutex is *locking* mechanism used to synchronize access to a
>> resource. Only one task (can be a thread or process based on OS
>> abstraction) can acquire the mutex. It means there is *ownership*
>> associated with mutex, and only the owner can release the lock
>> (mutex).
>>    -- A semaphore is *signaling* mechanism (“I am done, you can carry
>> on” kind of signal). For example, if you are listening songs (assume
>> it as one task) on your mobile and at the same time your friend calls
>> you, an interrupt is triggered upon which an interrupt service routine
>> (ISR) signals the call processing task to wakeup
>>
>>
>>> It is common case that semaphore "free" by default.
>>
>> With an implementation like..
>>      Semaphore>>forMutualExclusion
>>          ^self new signal
>>
>> we don't gain a lot, and its a misleading convenience method since its
>> doesn't provide the proper facility for mutual exclusion (and I do
>> conflate mutual exclusion == mutex).   Indeed, "Semaphore
>> forMutualExclusion + roll your ownership management + problems since
>> you weren't aware you needed to roll your own ownership management" is
>> not more convenient than "Mutex new"
>>
>> If you want to keep a convenience method for a pre signalled
>> Semaphore, then we could provide something like  "Semaphore
>> newSignalled"
>>
>>> And this name is explicitly said about this.
>>
>> Actually no.  It explicitly says it provides mutual exclusion - which
>> without ownership, it does not. It only implicitly provides a
>> pre-signalled Semaphore - you have to *know* of look at the source.
>>
>> [1]
>> https://blog.feabhas.com/2009/09/mutex-vs-semaphores-%E2%80%93-part-2-the-mutex/
>> [2] http://www.geeksforgeeks.org/mutex-vs-semaphore/
>>
>> cheers -ben
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Tudor Girba-2
Thanks!

Doru


> On Jan 6, 2016, at 12:56 AM, Ben Coman <[hidden email]> wrote:
>
> okay. I'll have a go.
> cheers -ben
>
> On Wed, Jan 6, 2016 at 3:28 AM, stepharo <[hidden email]> wrote:
>> Ben
>>
>> would you not be interested to write a little chapter on concurrency because
>> we would all benefit from it.
>> I can help but I'm pretty bad with concurrency.
>> I'm sure that doing that we will find a lot of things to improve in Pharo :)
>>
>> Stef
>>
>> Le 5/1/16 18:15, Ben Coman a écrit :
>>
>>> On Tue, Jan 5, 2016 at 11:59 PM, Denis Kudriashov <[hidden email]>
>>> wrote:
>>>>
>>>> 2016-01-05 16:06 GMT+01:00 Ben Coman <[hidden email]>:
>>>>>
>>>>> btw I have this growing suspicion that we should eliminate
>>>>> Semaphore>>critical: and Semaphore>>forMutualExclusion.   Semaphores
>>>>> on their own should *not* provide a facility for to protect critical
>>>>> regions since they suffer from accidental release [1] due to
>>>>> *lack*of*ownership*.
>>>>
>>>>
>>>> I have same feeling when analyse problem of critical: implementation. But
>>>> I
>>>> not think that Semaphore>>forMutualExclusion is bad.
>>>
>>> But Semaphore>>forMutualExclusion *encourages* people to believe
>>> semaphores can be used *on*their*own* for mutual exclusion, when they
>>> *shouldn't*.  You *must* add ownership.  If the mutual exclusion
>>> object doesn’t have ownership then, irrelevant of what it is called,
>>> it is not a mutex!!! [1].
>>>
>>> "Strictly speaking [2]...
>>>   -- A mutex is *locking* mechanism used to synchronize access to a
>>> resource. Only one task (can be a thread or process based on OS
>>> abstraction) can acquire the mutex. It means there is *ownership*
>>> associated with mutex, and only the owner can release the lock
>>> (mutex).
>>>   -- A semaphore is *signaling* mechanism (“I am done, you can carry
>>> on” kind of signal). For example, if you are listening songs (assume
>>> it as one task) on your mobile and at the same time your friend calls
>>> you, an interrupt is triggered upon which an interrupt service routine
>>> (ISR) signals the call processing task to wakeup
>>>
>>>
>>>> It is common case that semaphore "free" by default.
>>>
>>> With an implementation like..
>>>     Semaphore>>forMutualExclusion
>>>         ^self new signal
>>>
>>> we don't gain a lot, and its a misleading convenience method since its
>>> doesn't provide the proper facility for mutual exclusion (and I do
>>> conflate mutual exclusion == mutex).   Indeed, "Semaphore
>>> forMutualExclusion + roll your ownership management + problems since
>>> you weren't aware you needed to roll your own ownership management" is
>>> not more convenient than "Mutex new"
>>>
>>> If you want to keep a convenience method for a pre signalled
>>> Semaphore, then we could provide something like  "Semaphore
>>> newSignalled"
>>>
>>>> And this name is explicitly said about this.
>>>
>>> Actually no.  It explicitly says it provides mutual exclusion - which
>>> without ownership, it does not. It only implicitly provides a
>>> pre-signalled Semaphore - you have to *know* of look at the source.
>>>
>>> [1]
>>> https://blog.feabhas.com/2009/09/mutex-vs-semaphores-%E2%80%93-part-2-the-mutex/
>>> [2] http://www.geeksforgeeks.org/mutex-vs-semaphore/
>>>
>>> cheers -ben
>>>
>>>
>>
>>
>

--
www.tudorgirba.com
www.feenk.com

"Reasonable is what we are accustomed with."


Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen
In reply to this post by Henrik Sperre Johansen

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

signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen
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


signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov
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



MutexTest.st (7K) Download Attachment
Mutex.st (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov

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.

And I don't know how to fix it inside critical method. 

Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov
In reply to this post by Denis Kudriashov
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



Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

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




Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Ben Coman
On Thu, Jan 7, 2016 at 1:48 AM, Denis Kudriashov <[hidden email]> wrote:

> 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?

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.  Maybe the following
alternative would be required...

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

but I think my first proposal reads better.


OR.... we could have a primitive similar to ensure:

  ensure:
    | complete returnValue |
    <primitive: 198>
    returnValue := self valueNoContextSwitch.
    complete ifNil:[
      complete := true.
      aBlock value ].
    ^ returnValue

where I guess the primitive manipulates local variable "complete"
(??).  Then it could set that based on whether the primitive consumed
a signal or not...

  afterWaitSucceedsEnsure: aBlock
    | completeAndNeedSignal mutuallyExcludedValue |
    <primitive: 198b >
    mutuallyExcludedValue := self valueNoContextSwitch.
    completeAndNeedSignal ifNil:[
      completeAndNeedSignal := true.
      aBlock value.
      ].
    ^ mutuallyExcludedValue


Then we could just have
  critical: mutuallyExcludedBlock
    mutuallyExcludedBlock afterWaitSucceedsEnsure: [ self signal ].


but I still like my first proposal better.

cheers -ben


>
>
> 2016-01-06 14:48 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>> 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
>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov

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?
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov
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?

Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen
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




"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?



SignalUsingUnwindMachinery.cs (5K) Download Attachment
signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen
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?




signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov
Yes. When process suspend called during termination process has no more semaphore as suspendingList

2016-01-08 14:56 GMT+01:00 Henrik Johansen <[hidden email]>:
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?




Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen

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

Yes. When process suspend called during termination process has no more semaphore as suspendingList

Which is why in terminate I do
myList := self suspend.

So the semaphore *should* still be the suspendingList, just without the process actually on the list...
Seems to me something strange happens between setting activeProcess using execute:onBehalfOf: and when it reaches the ifCurtailed block, at that point the terminating thread is no longer the "active" process.

Cheers,
Henry


2016-01-08 14:56 GMT+01:00 Henrik Johansen <[hidden email]>:
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?






signature.asc (859 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Denis Kudriashov

2016-01-08 15:17 GMT+01:00 Henrik Johansen <[hidden email]>:
Which is why in terminate I do
myList := self suspend.

So the semaphore *should* still be the suspendingList, just without the process actually on the list...

I just test it in workspace.
sema := Semaphore new.
p := [sema wait] fork.
p suspend.
p suspendingList == nil
sema asArray isEmpty

Reply | Threaded
Open this post in threaded view
|

Re: [Ann] ReadWriteLock

Henrik Sperre Johansen

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


2016-01-08 15:17 GMT+01:00 Henrik Johansen <[hidden email]>:
Which is why in terminate I do
myList := self suspend.

So the semaphore *should* still be the suspendingList, just without the process actually on the list...

I just test it in workspace.
sema := Semaphore new.
p := [sema wait] fork.
p suspend.
p suspendingList == nil
sema asArray isEmpty


Of course, that's something entirely different...
if you do what I actually do in the modified terminate;
sema := Semaphore new.
p := [sema wait ] newProcess.
p resume.
1 second wait.
list := p suspend.
"this list is (re)assigned to myList in terminate"
list == nil
list == sema

You should get different answers.

Cheers,
Henry



signature.asc (859 bytes) Download Attachment
123