> On 21 Mar 2018, at 10:07 , Alistair Grant <[email protected]> wrote:
>
> But to help track down your issue, maybe temporarily replace the code with:
>
> <primitive: 'primitiveDirectoryLookup' module: 'FilePlugin' error:
> errorCode >
> errorCode ifNotNil: [self halt: errorCode asString].
> ^ #badDirectoryPath
>
>
> That should allow you to track down the issue a bit more.
>
> P.S. If the primitive really is missing, errorCode will be #'not found'.
I still got #badDirectoryPath, so I assumed that the primitive was there, and
was just returning the wrong answer. The question then became: why?
After some hours spent investigating, I figured out what is going on. Here is
the story.
The new file streams have behaviour that differs in several respects from the
old ones.
Here are three things that affected my application:
1. sending #readstream to an old file stream answered self; with the new ones,
I get a MNU.
2. closing a stream that that already been closed used to be a no-op; now it
causes a primitive failure walkback.
3. Repositioning a filestream stream doesn’t work. Unfortunately, sending
#position: doesn’t fail immediately, but the effect in my app (a compiler) is
that the file I’m reading appears to be empty.
Now for the interesting bit. Because the file appeared empty, my tests opened
the default prelude file. This also appeared to be empty, so it opened the
default prelude file again, while it was still open the first time. This
continued indefinitely — presumably, until some resource in the plugin was
exhausted. Once that happened, the plugin was permanently broken. Probably
the unwind machinery should have released the resource, but it didn’t, probably
because the resource is in the plugin, not in Pharo.
You can argue that (1) is the desired behaviour, but it is atypical. Sending
#asString to a String, #asNumber to a Number, or #asSet to a Set all return
self. I think that sending #readStream to a ReadStream should likewise answer
self. It used to.
You can argue that (2) ought to fail, and I’ll probably agree with you.
However, the failure should be a clean one (‘attempt to close a stream that has
already been closed’), not a primitive failure. And then /all/ streams should
behave that way, and there should be a #closeIfNecessary operation that does
what #close used to do. Incidentally, the predicate that tests whether a
stream is closed is named #closed; it should be named #isClosed.
(3), as already been discussed here, is just wrong. If a stream has a
#position: method, it should work. If we need another wrapper to implement it,
so be it. If #position: can’t be implemented for a particular kind of stream
(e.g., an interactive input stream), then the method either should not be
present at all, or should unconditionally raise an exception.
I have not investigated exactly how #position: fails, but I’m pretty sure that
this is the culprit. If I replace the `fileStream` by `fileStream contents
readStream`, everything works fine again.
As for the primitive failing permanently because it runs out of resources —
I’ll leave that to smarter people than me to figure out. At very least, it
should produce an intelligible ResourceExhausted exception.
Andrew
