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
["robust-forceChangesToDisk.1.cs"]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220325/807cea36/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: robust-forceChangesToDisk.1.cs
Type: application/octet-stream
Size: 2675 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220325/807cea36/attachment.obj>