• welcome

    ProcessVariable>>valueIfAbsent: bug (and fix)

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

    ProcessVariable>>valueIfAbsent: bug (and fix)

    Piotr Klibert-2
    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

    Reply | Threaded
    Open this post in threaded view
    |

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

    Holger Freyther
    Lovely find! Can you think of a testcase?

    thank you!
            holger

    > On 8. Jun 2021, at 23:06, Piotr Klibert <[hidden email]> 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
    >


    Reply | Threaded
    Open this post in threaded view
    |

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

    Piotr Klibert-2
    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 <[hidden email]> 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
    > >
    >
    >

    日韩在线旡码免费视频

    HoMEպߔƵ