Fix Windows write support (issue #1005)#1159
Open
riverdusty wants to merge 1 commit into
Open
Conversation
Soil could not be opened or written on Windows. Reproducing on a live Pharo image revealed three layered blockers, each only reachable after fixing the prior one: 1. fsync: BinaryFileStream>>fsync used POSIX fileno/fsync (absent on Windows). 2. file locking: OSPlatform had no Windows flock implementation. 3. multiple file open (ApptiveGrid#1005): the same path was opened in write mode from a second stream, which Windows rejects. Windows prerequisites (1 & 2) are ported from the unmerged integrate-windows-locking branch (those Soil-File files were unchanged on main since the merge-base): SoilWindowsFileLock (LockFileEx/UnlockFileEx + FlushFileBuffers), SoilWinOverlappedStruct, WinPlatform sync/lock, platform syncFile: dispatch in BinaryFileStream, and the Soil*FileLock renames. For ApptiveGrid#1005, add read-only stream support and route read-only file access through it so a path only ever has one write-mode handle: - SoilLockableStream readOnlyPath: / FileReference binaryReadOnlyStream (in-image locking only; a read-only handle cannot hold an OS write lock). - Journal recovery, journal query/replication (transactionJournalsStartingAt:do:), and the read-only journal visitor (SoilBasicVisitor>>visitJournalFragmentFile:) now open fragment files read-only and close them. - Recovery closes its read-only handle before the journal reopens for writing; cycleFragmentFileFrom: cycles from a read-only recovery file without flushing. - Backup reuses the already-open behavior-registry identifier index instead of reopening the same file. - Index commit (SoilNew{SkipList,BTree}IndexEntry>>commitIn:) reuses an already-open index instead of re-initializing it. Verified on Windows: create/write/close/reopen/write round-trip works; SoilBackupTest, SoilDatabaseJournalTest (incl. replication) and SoilTransactionTest pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Soil could not be opened or written on Windows. Reproducing on a live Pharo image revealed three layered blockers, each only reachable after fixing the prior one:
fsync—BinaryFileStream>>fsynccalled POSIXfileno/fsync(absent on Windows, givingSymbolNotFoundErroronmsvcrt).OSPlatform>>flockClasshad no Windows implementation.CannotDeleteFileException.Fixes #1005.
What changed
Windows prerequisites (1 & 2) — ported from the unmerged
integrate-windows-lockingbranch (thoseSoil-Filefiles were unchanged onmainsince the merge-base, so taken as-is):SoilWindowsFileLock(LockFileEx/UnlockFileEx+FlushFileBuffers),SoilWinOverlappedStruct,WinPlatformlock/syncBinaryFileStream>>fsyncnow delegates toOSPlatform>>syncFile:;fileno:/fsync:/syncFile:moved onto the platform classesUnixFileLock/MacOSFileLocktoSoilUnixFileLock/SoilMacOSFileLockIssue #1005 — read-only opens / single write-owner per path:
SoilLockableStream class>>readOnlyPath:+FileReference>>binaryReadOnlyStream(in-image locking only — a read-only handle cannot hold an OS write lock, and these reads are of a consistent snapshot)transactionJournalsStartingAt:do:), and the read-only journal visitor (SoilBasicVisitor>>visitJournalFragmentFile:) now open fragment files read-only and close themcycleFragmentFileFrom:cycles from a read-only recovery file without flushing/syncing itidentifierindex instead of opening the same file a second timeSoilNewSkipListIndexEntry/SoilNewBTreeListIndexEntry>>commitIn:) reuses an already-open index instead of re-initializeFilesystemVerification
Verified end-to-end on Windows (Pharo image): create -> write -> close -> reopen -> write -> reopen -> read round-trips with data intact. Suites green:
SoilBackupTest(7/7),SoilDatabaseJournalTestincl. replication (9/9),SoilTransactionTest(9/9);testCheckpointEmptyRecordsToCommitandtestConsistencypass.Notes for reviewers
destroythen hitsCannotDeleteFileException. Every such test passes when run individually.SoilLockableStreamTestalso has a pre-existing, unrelated Windows path bug.Generated with Claude Code