On Tue, Jan 26, 2021 at 2:45 PM Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Marcel, Hi All,

On Mon, Jan 25, 2021 at 3:26 AM Marcel Taeumel <marcel.taeumel@hpi.de> wrote:
It's simply not thread-safe.

Hmm... in particular, there is a side effect (i.e. layout update) in the window's pluggable text morph. Even if #forceUpdate is false.

In #endEntry, we could check for "Processor activeProcess == Project current uiProcess" to then use "Project current addDeferredUIMessage..." automatically.

That gave me a scare.  At first I thought that the effectiveProcess changes to ProcessorScheduler>>activeProcess could have made that expression no longer thread safe.  But it isn't anyway, and the above is very bad style.  Let me explain.

The suspension points in the VM are 
- the first bytecode of any non-quick method (following a primitive if it has one)
- the first bytecode of any block
- a backward branch

In the JIT there are fewer suspension points:
- the first bytecode of any method containing a send other than #== and/or #class (and given that ifNil:ifNotNil: is inlined into a send of #== and jumps, ifNil: sends don't count)
- the first bytecode of any block containing a send other than #== and/or #class
- a backward branch

So given

ProcessorScheduler>>#activeProcess
        "suspension point here on an interpreter VM, and the first time in the JIT VM"
^activeProcess effectiveProcess

Process>>#effectiveProcess
        "suspension point here on an interpreter VM, and the first time in the JIT VM"
^effectiveProcess ifNil: [self]

Project class>>#current
        "suspension point here on an interpreter VM, and the first time in the JIT VM"
^CurrentProject

MVCProject>>#uiProcess
        "suspension point here on an interpreter VM, and the first time in the JIT VM"
^ world activeControllerProcess

MorphicProject>>#uiProcess
        "no suspension point here on any VM obeying the blue-book spec"
^uiProcess

we can see that "Processor activeProcess == Project current uiProcess" is not thread-safe in the interpreter; there is a suspension point in Project current before CurrentProject is returned (but not in the JIT); there is a suspension point in MVC in uiProcess.  There is a suspension point in ProcessorScheduler>>#activeProcess before the send of effectiveProcess to activeProcess, but since no data is fetched until the process resumes this is harmless.  However, were the expression written as "Project current uiProcess == Processor activeProcess" then this would no longer be harmless.

So on all VMs this is not reliably thread-safe.  Even on the JIT VM the first use of any method (except a doit) is interpreted.  Once the JIT VM, once it has started up and the methods have been jitted, "Processor activeProcess == Project current uiProcess" is thread-safe on Morphic, but not on MVC.  So we really shouldn't be using things like this.  They'll appear to work and in very rare circumstances they won't.  So let's please use the proper construct, Mutex.


But that makes me realize that the effectiveProcess changes are unsafe on an interpreter VM (and hence we're on;y getting away with it on the JIT VM).  Se really need some way of saying that ProcessorScheduler>>#activeProcess is atomic, e.g.

Process>>#effectiveProcess
        <noContextSwitch>
^effectiveProcess ifNil: [self]

But I don't know how to implement this right now.  Note that we do have 

BlockClosure>>#valueNoContextSwitch
"An exact copy of BlockClosure>>value except that this version will not preempt
the current process on block activation if a higher-priority process is runnable.
Primitive. Essential."
<primitive: 221>
numArgs ~= 0 ifTrue:
[self numArgsError: 0].
self primitiveFailed

FullBlockClosure>>#valueNoContextSwitch
"An exact copy of BlockClosure>>value except that this version will not preempt
the current process on block activation if a higher-priority process is runnable.
Primitive. Essential."
<primitive: 209>
numArgs ~= 0 ifTrue:
[self numArgsError: 0].
self primitiveFailed

but the VM uses the primitive numbers to control context switching.  We don't have a mechanism the VM can use to label a normal method such as Process>>#effectiveProcess as not to allow an interrupt.

Doh!  We have primitive numbers.  We already use primitive 19 as a "simulation guard" in the debugger machinery as a "simulation guard" to avoid I'm not sure what.  Any unused numeric primitive could be used as a marker to avoid context switches.  I propose that we use it, hiding it behind a suitable pragma for syntactic sugar (i.e. <noContextSwitch>).  123 used to be primitiveValueUninteruptibly, which we no longer use.  I propose we use this.

Best,
Marcel

Am 23.01.2021 21:05:05 schrieb Levente Uzonyi <leves@caesar.elte.hu>:

On Fri, 22 Jan 2021, tim Rowledge wrote:

>
>
>> On 2021-01-22, at 5:26 PM, David T. Lewis wrote:
>>
>> On Fri, Jan 22, 2021 at 02:03:10PM -0600, jaromir wrote:
>>> I ran the following in Squeak 5.3:
>>>
>>> TranscriptStream forceUpdate: false.
>>> [ 10000 timesRepeat: [ Transcript show: 'x' ] ] forkAt: 39
>>>
>>> When the Transcript window fills up, 'Message not understood' and 'Assertion
>>> failure' appear. I'm wondering is this a bug? Thanks. Jaromir
>>>
>>>
>>>
>>
>> It certainly does appear to be a bug.
>
> We had some discussion about it not so long ago; you can't use the Transcript to log bugs from Seaside very well, for example. Background processes, mutexs, that sort of thing. IIRC Levente offered an explanation?

It's simply not thread-safe.
If you want it to be thread-safe, you can pass your message to the UI
process to show it. Make sure your message is computed in its own process
and is passed as a precomputed string to avoid other kinds of race
conditions. E.g.:

| theMessage |
theMessage := 'Something very {1}.' format: { 'important' }.
Project current addDeferredUIMessage: [ Transcript show: theMessage; cr ].


Levente

>
>
> tim
> --
> tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim
> The hardness of the butter is proportional to the softness of the bread.




--
_,,,^..^,,,_
best, Eliot


--
_,,,^..^,,,_
best, Eliot