Re: ProcessVariable>>valueIfAbsent: bug (and fix)

[Piotr Klibert]

Wouldn't something like this suffice?

    TestCase subclass: ProcessVariableTestCase [
        | procvar |

        setUp [
            procvar := ProcessVariable new.
        ]

        testIfAbsent [
            | val |
            val := procvar value.
            self should: [ val isNil ].
            val := procvar valueIfAbsent: [ true ].
            self should: [ val = true ].
        ]
    ]

It should at least catch possible regressions. It would be good to test the 
behavior with multiple processes, but I think this is outside the scope of this 
bug: it's reproducible in a single process, and there should be no difference 
in behavior with multiple processes, as long as the value is not set in any of 
them. 


Actually, I happened upon this when trying to use DynamicVariable (which uses 
ProcessVariable under the hood). I'm writing a REPL accessible via a 
SocketServer, for a few reasons (one of them: the local, default REPL seems to 
block other running processes when waiting for input, not sure if it's a bug?), 
and I wanted to make Transcript process-local. Each connection lives in its own 
process, so to make all the output go to the socket instead of stdout, I did 
something like this:

    DynamicVariable subclass: LocalTranscript [
        LocalTranscript class >> value [
            ^super valueIfAbsent: [ StdOutTranscript ].
        ]
    ]

    Object subclass: DynamicTranscript [
        doesNotUnderstand: aMessage [
            aMessage sendTo: LocalTranscript value.
        ]
    ]

    Smalltalk at: #StdOutTranscript put: (Smalltalk at: #Transcript).
    Smalltalk at: #Transcript put: DynamicTranscript new.

    Object subclass: CommandHandler [
        primHandleCommand: cmd withOutputTo: aTextCollector [ 
            lastLine := cmd.
            LocalTranscript use: aTextCollector during: [
                self handleEval printString displayNl.
            ]
        ]
    ]

The bug manifested itself when I tried to print something outside of the 
LocalTranscript>>#use:during: (some logging from the server itself).
 

On Wed, Jun 9, 2021, at 10:18, Holger Freyther wrote:
> Lovely find! Can you think of a testcase?
> 
> thank you!
>       holger
> 
> > On 8. Jun 2021, at 23:06, Piotr Klibert <piotrklibert@fastmail.com> wrote:
> > 
> > In the previous thread I mentioned a problem in ProcessVariable I 
> > encountered while trying out VisualGST. Turns out I was mistaken, the 
> > problem was triggered by something else, as I wasn't able to reproduce it 
> > after a fresh rebuild. Still, the bug is there for sure: the 
> > `valueIfAbsent` never evaluates the block it gets as an argument. Here's 
> > the patch:
> > 
> > 
> >    diff --git a/kernel/ProcEnv.st b/kernel/ProcEnv.st
> >    index d3780748..fa695641 100644
> >    --- a/kernel/ProcEnv.st
> >    +++ b/kernel/ProcEnv.st
> >    @@ -77,7 +77,7 @@ setting for the current process.'>
> >         valueIfAbsent: aBlock [
> >            "Return the value of this variable in the current process."
> >            <category: 'accessing'>
> >    -       ^Processor activeProcess environment at: self key ifAbsent: [ 
> > nil ]
> >    +       ^Processor activeProcess environment at: self key ifAbsent: 
> > aBlock.
> >         ]
> > 
> >         value [
> > 
> > 
> > (the method comment should probably also be changed a bit, now that I look 
> > at it...)
> > 
> > 
> > Best regard,
> > Piotr Klibert
> > 
> 
>