[Pharo70 alpha] Store settings raise a DNU primFlush

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

[Pharo70 alpha] Store settings raise a DNU primFlush

Stephane Ducasse-3
Hi

I do not know if this is related to the OldStreamCleaningEffort but
when I press store settings in the Settings browser I get a primitive
failed.

https://pharo.fogbugz.com/f/cases/21699/Store-settings-raise-a-DNU-primFlush

Stef

Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

alistairgrant
Hi Stef & Sven,

On 13 April 2018 at 20:38, Stephane Ducasse <[hidden email]> wrote:
> Hi
>
> I do not know if this is related to the OldStreamCleaningEffort but
> when I press store settings in the Settings browser I get a primitive
> failed.
>
> https://pharo.fogbugz.com/f/cases/21699/Store-settings-raise-a-DNU-primFlush


This is because the stream created in
SystemSettingsPersistence>>storeExactStoredSettings: is closed twice,
first in SettingsStonWriter>>store and then in
SystemSettingsPersistence>>storeExactStoredSettings:.

Previous versions of file streams have allowed the stream #close to be
called multiple times, however ZnBufferedReadWriteStream doesn't allow
this.

I think that since an already open stream is handed to
SettingsStonWriter, it shouldn't be closing the stream.  That will
resolve this particular problem.

It can also be argued that ZnBufferedReadWriteStream and co. should
allow streams to be closed multiple times for backward compatibility.
Sven?

Cheers,
Alistair

Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

Sven Van Caekenberghe-2


> On 13 Apr 2018, at 22:26, Alistair Grant <[hidden email]> wrote:
>
> Hi Stef & Sven,
>
> On 13 April 2018 at 20:38, Stephane Ducasse <[hidden email]> wrote:
>> Hi
>>
>> I do not know if this is related to the OldStreamCleaningEffort but
>> when I press store settings in the Settings browser I get a primitive
>> failed.
>>
>> https://pharo.fogbugz.com/f/cases/21699/Store-settings-raise-a-DNU-primFlush
>
>
> This is because the stream created in
> SystemSettingsPersistence>>storeExactStoredSettings: is closed twice,
> first in SettingsStonWriter>>store and then in
> SystemSettingsPersistence>>storeExactStoredSettings:.
>
> Previous versions of file streams have allowed the stream #close to be
> called multiple times, however ZnBufferedReadWriteStream doesn't allow
> this.
>
> I think that since an already open stream is handed to
> SettingsStonWriter, it shouldn't be closing the stream.  That will
> resolve this particular problem.
>
> It can also be argued that ZnBufferedReadWriteStream and co. should
> allow streams to be closed multiple times for backward compatibility.
> Sven?

Yes, OK, making #close idempotent is good. If we add #closed to ZnBufferedReadStream we can add a

  self closed ifFalse: [ .. ]

guard in #close.

> Cheers,
> Alistair
>


Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

Stephane Ducasse-3
Thanks guy.
I was dead tired. Did not have the energy to look went to sleep.


On Fri, Apr 13, 2018 at 10:44 PM, Sven Van Caekenberghe <[hidden email]> wrote:

>
>
>> On 13 Apr 2018, at 22:26, Alistair Grant <[hidden email]> wrote:
>>
>> Hi Stef & Sven,
>>
>> On 13 April 2018 at 20:38, Stephane Ducasse <[hidden email]> wrote:
>>> Hi
>>>
>>> I do not know if this is related to the OldStreamCleaningEffort but
>>> when I press store settings in the Settings browser I get a primitive
>>> failed.
>>>
>>> https://pharo.fogbugz.com/f/cases/21699/Store-settings-raise-a-DNU-primFlush
>>
>>
>> This is because the stream created in
>> SystemSettingsPersistence>>storeExactStoredSettings: is closed twice,
>> first in SettingsStonWriter>>store and then in
>> SystemSettingsPersistence>>storeExactStoredSettings:.
>>
>> Previous versions of file streams have allowed the stream #close to be
>> called multiple times, however ZnBufferedReadWriteStream doesn't allow
>> this.
>>
>> I think that since an already open stream is handed to
>> SettingsStonWriter, it shouldn't be closing the stream.  That will
>> resolve this particular problem.
>>
>> It can also be argued that ZnBufferedReadWriteStream and co. should
>> allow streams to be closed multiple times for backward compatibility.
>> Sven?
>
> Yes, OK, making #close idempotent is good. If we add #closed to ZnBufferedReadStream we can add a
>
>   self closed ifFalse: [ .. ]
>
> guard in #close.
>
>> Cheers,
>> Alistair
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

Sean P. DeNigris
Administrator
In reply to this post by Sven Van Caekenberghe-2
Sven Van Caekenberghe-2 wrote
> If we add #closed to ZnBufferedReadStream…

Would #isClosed be more consistent with the rest of the system?



-----
Cheers,
Sean
--
Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html

Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

Denis Kudriashov
+1 to deprecate #closed

2018-04-14 16:24 GMT+02:00 Sean P. DeNigris <[hidden email]>:
Sven Van Caekenberghe-2 wrote
> If we add #closed to ZnBufferedReadStream…

Would #isClosed be more consistent with the rest of the system?



-----
Cheers,
Sean
--
Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html


Reply | Threaded
Open this post in threaded view
|

Re: [Pharo70 alpha] Store settings raise a DNU primFlush

Sven Van Caekenberghe-2


> On 14 Apr 2018, at 16:35, Denis Kudriashov <[hidden email]> wrote:
>
> +1 to deprecate #closed

Talk to Guille, he just added it recently, IIUC

(I do agree about #isClosed being better, but we can't keep changing things all the time)

> 2018-04-14 16:24 GMT+02:00 Sean P. DeNigris <[hidden email]>:
> Sven Van Caekenberghe-2 wrote
> > If we add #closed to ZnBufferedReadStream…
>
> Would #isClosed be more consistent with the rest of the system?
>
>
>
> -----
> Cheers,
> Sean
> --
> Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
>
>