> On 21 Mar 2018, at 10:07 , Alistair Grant <
[hidden email]> 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
Locked
