[squeak-dev] Review Request: robust-forceChangesToDisk.1.cs

christoph.thiede at student.hpi.uni-potsdam.de christoph.thiede at student.hpi.uni-potsdam.de
Fri Apr 1 11:10:10 UTC 2022


Thanks for the feedback! Merged via Files-ct.193, System-ct.1333, and Tests-ct.481. :-)

I am curious whether we will see any further 'getPrimPosition failed' errors for other reasons in the future, though. :D

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2022-03-31T09:35:14+02:00, marcel.taeumel at hpi.de wrote:

> Wohoo! This issue has been bothering me for ages! Yes, please.
> 
> Best,
> Marcel
> Am 25.03.2022 14:49:16 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:
> Please review. :-)
> 
> Best,
> Christoph
> 
> =============== Summary ===============
> 
> Change Set:        robust-forceChangesToDisk
> Date:            25 March 2022
> Author:            Christoph Thiede
> 
> Increases the robustness of Smalltalk forceChangesToDisk. When this method is interrupted, make sure to maintain the invariant that the source file is always opened and set to the right position. In the course of this, also makes StandardFileStream>>#open:forWrite: more robust. Adds a smoke/regression test.
> 
> For context: Interrupting Smalltalk forceChangesToDisk is not a fictional scenario. In combination with a slow file system access, I could reproduce this many times when running StringTest>>#testPercentEncodingJa with a too small timeout.
> 
> =============== Diff ===============
> 
> SmalltalkImage>>forceChangesToDisk {sources, changes log} · ct 3/25/2022 14:17 (changed)
> forceChangesToDisk
>     "Ensure that the changes file has been fully written to disk by closing and re-opening it. This makes the system more robust in the face of a power failure or hard-reboot."
> 
>     | changesFile |
>     changesFile := SourceFiles at: 2.
>     (changesFile isKindOf: FileStream) ifTrue: [
>         changesFile flush.
> -         SecurityManager default hasFileAccess ifTrue:[
> -             changesFile close.
> -             changesFile open: changesFile name forWrite: true].
> -         changesFile setToEnd.
> -     ].
> +         SecurityManager default hasFileAccess
> +             ifTrue: [
> +                 [changesFile close.
> +                 changesFile open: changesFile name forWrite: true.
> +                 changesFile setToEnd]
> +                     ifCurtailed: [
> +                         changesFile closed ifFalse: [changesFile close].
> +                         changesFile open: changesFile name forWrite: true.
> +                         changesFile setToEnd]]
> +             ifFalse: [changesFile setToEnd]].
> 
> 
> SmalltalkImageTest>>testForceChangesToDiskRobustness {tests} · ct 3/25/2022 14:23
> + testForceChangesToDiskRobustness
> +     <timeout: 11 "seconds">
> +     <fatal "regression test! Unless this bug is fixed, this may damage your image.">
> +
> +     50 timesRepeat:
> +         [[[Smalltalk forceChangesToDisk] repeat] valueWithin: 0.1 seconds onTimeout: []].
> +     
> +     self shouldnt: [Smalltalk logChange: ('Test from {1}' format: {self})] raise: Error.
> 
> StandardFileStream>>open:forWrite: {open/close} · ct 3/25/2022 14:42 (changed)
> open: fileName forWrite: writeMode
>     "Open the file with the given name. If writeMode is true, allow writing, otherwise open the file in read-only mode."
>     "Changed to do a GC and retry before failing ar 3/21/98 17:25"
> -     | f |
> +     | f newFileID |
>     f := fileName asVmPathName.
> -
> -     fileID := StandardFileStream retryWithGC:[self primOpen: f writable: writeMode]
> -                     until:[:id| id notNil]
> -                     forFileNamed: fileName.
> +     
> +     fileID := nil.
> +     fileID := [StandardFileStream
> +         retryWithGC:
> +             [newFileID := nil.
> +             newFileID := self primOpen: f writable: writeMode]
> +         until: [:id | id notNil]
> +         forFileNamed: fileName]
> +             ifCurtailed:
> +                 [newFileID ifNotNil: [self primClose: newFileID]].
>     fileID ifNil: [^ nil]. "allows sender to detect failure"
>     name := fileName.
>     self register.
>     rwmode := writeMode.
>     buffer1 := String new: 1.
> -     self enableReadBuffering
> -     
> +     self enableReadBuffering.
> 
> ["robust-forceChangesToDisk.1.cs"]
> 
> ---
> Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]
> ["robust-forceChangesToDisk.1.cs"]
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220331/14a09809/attachment.html>
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220401/071489f0/attachment.html>


More information about the Squeak-dev mailing list