Eliot I found that there are no methods for simulation code (if it right name for it). So stepping over new primitives failed (which is infinite recursion for Pharo case)

Context>>doPrimitive: primitiveIndex method: meth receiver: aReceiver args: arguments

...

"Mutex>>primitiveEnterCriticalSection

Mutex>>primitiveTestAndSetOwnershipOfCriticalSection"

(primitiveIndex = 186 or: [primitiveIndex = 187]) ifTrue:

[| active effective |

active := Processor activeProcess.

effective := active effectiveProcess.

"active == effective"

value := primitiveIndex = 186

ifTrue: [aReceiver primitiveEnterCriticalSectionOnBehalfOf: effective]

ifFalse: [aReceiver primitiveTestAndSetOwnershipOfCriticalSectionOnBehalfOf: effective].

^(self isPrimFailToken: value)

ifTrue: [value]

ifFalse: [self push: value]].

How this methods should be implemented?

2016-01-11 13:59 GMT+01:00 Denis Kudriashov <dionisiydk@gmail.com>:

Hello.

I publish slice 17373.

I introduce LockOwnership class which implements VM primitives as:
- acquire
- tryAcquire
- release
(It comment saves copyright from CriticalSection)

Mutext uses it as ownership instance variable to implement critical methods correctly:
- critical:
- critical:ifLocked:
- critical:ifError:
- critical:ifCurtailed:

For the integration process old Mutex instance variables are not removed. So loading this code should not broke current Mutex instances. But when we integrate it Mutex will have only variable ownership.

If you look critical implementation you will understand why I not like semantic of primitives. it should be inverted in future. #acquire should return true when ownership is acquired right now.

Process>>terminate now detects waiting on LockOwnership and ask it to handle wait termination.Then LockOwnership inject right value into lock state variable. Such variables should be marked with special pragma 
    <lockAt: #ownershipVariableName tracksStateAt: 1> "index of local variable"
Method can contain mulpible pragmas to reference all ownerships in method. ReadWriteLock for example needs this.

2016-01-11 13:30 GMT+01:00 Henrik Johansen <henrik.s.johansen@veloxit.no>:

 

On 08 Jan 2016, at 4:25 , Ben Coman <btc@openInWorld.com> wrote:

On Fri, Jan 8, 2016 at 9:39 PM, Ben Coman <btc@openinworld.com> wrote:

On Fri, Jan 8, 2016 at 5:42 PM, stephane ducasse
<stephane.ducasse@gmail.com> wrote:

I have a (stupid) question.
Is the code running without the primitives?
Are the code below the primitives correct?
I asked that because we can have 100 eyes and brains on the smalltalk level and far less on the VM primitive level.

Because:
1. Concurrency bugs can be subtle and the *exact* conditions can be
hard to reproduce for debugging.  For example, the solution to a
couple of problems with Delay [1] [2] were solved by moving away from
Semaphore>>critical: to use signalling.

2. The in-image atomicity of determining whether a signal was actually
consumed or not during process suspension/termination is awkward.  Its
seems hard to *really* know for sure it right (but I haven't looked in
depth into Denis' latest proposals.)

3. The existing in-image implementation of Semaphore>>critical messes
around in  Process>>terminate in a *special* way that is not easy for
those 100 eyes to understand. For example, I personally am not
comfortable with understanding how the special Semaphore handling in
Process>>terminate works, but I can easily follow how
primitiveEnterCriticalSection just looking at the code [3].

Points 2 & 3 might possibly be addressed by having new primitiveWaitReturned
*always* return true, so if the process is terminated while waiting,
the assignment to signalConsumed doesn't occur...

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

where primitiveWait (https://git.io/vuDjd) is copied
and (just guessing) the marked line added...

 primitiveWaitReturned
     | sema excessSignals activeProc inInterpreter |
     sema := self stackTop. "rcvr"
"==>>" self pop: argumentCount + 1 thenPush: objectMemory trueObject. "<<=="
    excessSignals := self fetchInteger: ExcessSignalsIndex ofObject: sema.
     excessSignals > 0
         ifTrue:
           [self storeInteger: ExcessSignalsIndex
                       ofObject: sema
                       withValue: excessSignals - 1]
         ifFalse:
           inInterpreter := instructionPointer >= objectMemory startOfMemory.
           activeProc := self activeProcess.
           self addLastLink: activeProc toList: sema.
           self transferTo: self wakeHighestPriority from: CSWait.
           self
forProcessPrimitiveReturnToExecutivePostContextSwitch: inInterpreter]

which I guess could be added quickly if Esteban could compile the
latest pharo-spur-vm ;)

cheers -ben

Won't work, there's no guarantee thread has actually ran and signalConsumed been assigned the primitive result after Semaphore resumed the waiting thread, before  a higher priority thread runs and terminates it. (which is exactly the case handled by special code in #terminate)

Cheers,

Henry