Fix Windows write support (issue #1005)

src/Soil-Core-Tests/SoilTest.class.st

@@ -10,7 +10,7 @@ Class {
 { #category : #accessing }
 SoilTest class >> classNamesNotUnderTest [
 	"we for now ignore flock as this is platform specific"
-	^ #(#MacOSFileLock #UnixFileLock)
+	^ #(#SoilMacOSFileLock #SoilUnixFileLock)
 ]
 
 { #category : #accessing }

src/Soil-Core/SoilBackupVisitor.class.st

@@ -54,11 +54,17 @@ SoilBackupVisitor >> copyCluster: aSoilPersistentClusterVersion [
 { #category : #visiting }
 SoilBackupVisitor >> copyIndexAt: indexId segment: segmentId [
 
-	| sourceSegment sourceIndex targetSegment targetIndex iterator assoc |
+	| sourceSegment sourceIndex targetSegment targetIndex iterator assoc isBehaviorIndex |
+	"the behavior registry (identifier index of the meta segment) is already open
+	via soil behaviorRegistry. Reuse that open index instead of opening the same
+	file a second time through the index manager, which fails on Windows"
+	isBehaviorIndex := (segmentId = 0) and: [ indexId = #identifier ].
 	sourceSegment := soil objectRepository segmentAt: segmentId.
-	sourceIndex := sourceSegment indexManager
-		               at: indexId
-		               ifAbsent: [ ^ self indexNotFound: indexId ].
+	sourceIndex := isBehaviorIndex
+		               ifTrue: [ soil behaviorRegistry index ]
+		               ifFalse: [ sourceSegment indexManager
+				               at: indexId
+				               ifAbsent: [ ^ self indexNotFound: indexId ] ].
 
 	"create an index of same kind and configuration in the target database"
 	targetSegment := target objectRepository segmentAt: segmentId.
@@ -77,7 +83,10 @@ SoilBackupVisitor >> copyIndexAt: indexId segment: segmentId [
 	targetIndex
 		flush;
 		close.
-	sourceSegment indexManager removeIndexWithId: indexId
+	"do not close the behavior registry index here, it is owned by soil and
+	still in use; only release indexes we opened through the index manager"
+	isBehaviorIndex ifFalse: [
+		sourceSegment indexManager removeIndexWithId: indexId ]
 ]
 
 { #category : #visiting }
@@ -153,10 +162,14 @@ SoilBackupVisitor >> visitJournalFragmentFile: aSoilJournalFragmentFile [
 ]
 
 { #category : #visiting }
-SoilBackupVisitor >> visitMetaSegment: aSoilMetaSegment [ 
+SoilBackupVisitor >> visitMetaSegment: aSoilMetaSegment [
 	super visitMetaSegment: aSoilMetaSegment.
-	self copyIndexAt: #identifier segment: 0. 
-	^ aSoilMetaSegment  
+	"the target's behavior registry opened the identifier index when the target
+	filesystem was initialized. Close it first so copyIndexAt: can rebuild that
+	index without opening the same file a second time (which fails on Windows)"
+	target behaviorRegistry close.
+	self copyIndexAt: #identifier segment: 0.
+	^ aSoilMetaSegment
 ]
 
 { #category : #visiting }

src/Soil-Core/SoilBasicVisitor.class.st

@@ -79,11 +79,14 @@ SoilBasicVisitor >> visitIndexManager: aSoilIndexManager [
 ]
 
 { #category : #visiting }
-SoilBasicVisitor >> visitJournalFragmentFile: aSoilJournalFragmentFile [ 
-	aSoilJournalFragmentFile open.
+SoilBasicVisitor >> visitJournalFragmentFile: aSoilJournalFragmentFile [
+	"only read the fragment file. Opening read-only avoids a second write-mode
+	open of a file the journal may already hold open for writing (fails on
+	Windows). SoilBackupVisitor overrides this to copy the file instead"
+	aSoilJournalFragmentFile openReadOnly.
 	[ self visitAll: aSoilJournalFragmentFile transactionJournals ]
 		ensure: [ aSoilJournalFragmentFile close ].
-	^ aSoilJournalFragmentFile 
+	^ aSoilJournalFragmentFile
 ]
 
 { #category : #visiting }

src/Soil-Core/SoilDatabaseRecovery.class.st

@@ -61,11 +61,10 @@ SoilDatabaseRecovery >> readFragmentFileProtected: aSoilFragmentFile [
 			(journal lastFileNumber = aSoilFragmentFile fileNumber)
 				ifFalse: [ SoilDatabaseIsInconsistent signal: 'after a truncated file there should not be another one' ].
 			"we currently have no way of reading behind a truncated piece of a fragment. We cycle
-			the fragment file in order to write new entries to a new file. Future reads can just 
-			skip a fragment file on truncated reads"
-			journal 
-				currentFragmentFile: aSoilFragmentFile;
-				cycleFragmentFile ]
+			the fragment file in order to write new entries to a new file. Future reads can just
+			skip a fragment file on truncated reads. The fragment file was opened read-only for
+			recovery, so we cycle from it without flushing/syncing it"
+			journal cycleFragmentFileFrom: aSoilFragmentFile ]
 ]
 
 { #category : #private }
@@ -102,16 +101,19 @@ SoilDatabaseRecovery >> recover [
 	checkpointEntry := [ SoilJournalEntry readFrom: fragmentFile stream ]
 		on: Error 
 		do: [ :error | 
-			"in case of a bogus checkpoint position we cannot recover the file but 
-			create a new one and continue"
-			journal 
-				currentFragmentFile: fragmentFile;
-				cycleFragmentFile.
+			"in case of a bogus checkpoint position we cannot recover the file but
+			create a new one and continue. The fragment file was opened read-only,
+			so we cycle from it without flushing/syncing it"
+			journal cycleFragmentFileFrom: fragmentFile.
 			soil checkpoint.
 			^ self ].
-	"If the last checkpoint was successful we are at the end of the file and 
-	can return because the database is in a sane state"
-	fragmentFile atEnd ifTrue: [ ^ self ].
+	"If the last checkpoint was successful we are at the end of the file and
+	can return because the database is in a sane state. Close the read-only
+	recovery handle so the journal can later open the file for writing (a second
+	open in write mode while this one is still open fails on Windows)"
+	fragmentFile atEnd ifTrue: [
+		fragmentFile close.
+		^ self ].
 
 	"the fragment file contains more entries after the last checkpoint. Read the
 	current fragment file to its end and apply all transaction logs/checkpoints found"

src/Soil-Core/SoilJournalFragmentFile.class.st

@@ -239,7 +239,16 @@ SoilJournalFragmentFile >> inspectionTransactionJournals: aBuilder [
 { #category : #'open/close' }
 SoilJournalFragmentFile >> open [
 	self isOpen ifTrue: [ self error: 'File already open' ].
-	stream := SoilLockableStream path: path 
+	stream := SoilLockableStream path: path
+]
+
+{ #category : #'open/close' }
+SoilJournalFragmentFile >> openReadOnly [
+	"open the fragment file in read-only mode. Used during recovery which only
+	reads the file. This avoids opening the same path in write mode from a
+	second stream while the journal holds it open for writing (fails on Windows)"
+	self isOpen ifTrue: [ self error: 'File already open' ].
+	stream := SoilLockableStream readOnlyPath: path
 ]
 
 { #category : #accessing }

src/Soil-Core/SoilNewBTreeListIndexEntry.class.st

@@ -14,6 +14,10 @@ SoilNewBTreeListIndexEntry >> commitIn: soil recovery: aBoolean [
 
 	| index indexManager |
 	indexManager := (soil objectRepository segmentAt: segment) indexManager.
+	"if the index is already open (e.g. repeated commits of the same new index via
+	commitAndContinue) reuse it instead of opening the same file a second time,
+	which fails on Windows"
+	(indexManager indexes at: id ifAbsent: [ nil ]) ifNotNil: [ :existing | ^ existing ].
 	index := SoilBTree new
 		path: (indexManager pathFor: id);
 		initializeFilesystem;

src/Soil-Core/SoilNewSkipListIndexEntry.class.st

@@ -17,6 +17,10 @@ SoilNewSkipListIndexEntry >> commitIn: soil recovery: aBoolean [
 
 	| index indexManager |
 	indexManager := (soil objectRepository segmentAt: segment) indexManager.
+	"if the index is already open (e.g. repeated commits of the same new index via
+	commitAndContinue) reuse it instead of opening the same file a second time,
+	which fails on Windows"
+	(indexManager indexes at: id ifAbsent: [ nil ]) ifNotNil: [ :existing | ^ existing ].
 	index := SoilSkipList new
 		path: (indexManager pathFor: id);
 		initializeFilesystem;

src/Soil-Core/SoilPersistentDatabaseJournal.class.st

@@ -64,13 +64,24 @@ SoilPersistentDatabaseJournal >> currentFragmentFile: aSoilJournalFragmentFile [
 SoilPersistentDatabaseJournal >> cycleFragmentFile [
 	| nextFileNumber |
 	nextFileNumber := (self fileNumberFrom: currentFragmentFile filename) + 1.
-	currentFragmentFile 
+	currentFragmentFile
 		flush;
 		writeContentsToDisk;
 		close.
 	currentFragmentFile := self createFragmentFile: (self filenameFrom: nextFileNumber)
 ]
 
+{ #category : #fragmentfile }
+SoilPersistentDatabaseJournal >> cycleFragmentFileFrom: aSoilJournalFragmentFile [
+	"cycle to a new writable fragment file based on a fragment file that was
+	opened read-only during recovery. The read-only file is only closed (it must
+	not be flushed/synced) before the next writable fragment file is created"
+	| nextFileNumber |
+	nextFileNumber := (self fileNumberFrom: aSoilJournalFragmentFile filename) + 1.
+	aSoilJournalFragmentFile close.
+	currentFragmentFile := self createFragmentFile: (self filenameFrom: nextFileNumber)
+]
+
 { #category : #enumerating }
 SoilPersistentDatabaseJournal >> entriesDo: aBlock [
 	self fragmentFiles reverse do: [ :file |
@@ -267,8 +278,19 @@ SoilPersistentDatabaseJournal >> openFragmentFile: filename [
 ]
 
 { #category : #'instance creation' }
-SoilPersistentDatabaseJournal >> openFragmentFileNumber: anInteger [ 
-	^ self openFragmentFile: (self filenameFrom: anInteger)
+SoilPersistentDatabaseJournal >> openFragmentFileNumber: anInteger [
+	^ self openFragmentFileReadOnly: (self filenameFrom: anInteger)
+]
+
+{ #category : #'instance creation' }
+SoilPersistentDatabaseJournal >> openFragmentFileReadOnly: filename [
+	"open a fragment file in read-only mode. Used by recovery which only reads
+	the file, so it does not request write access to a path that may already be
+	open for writing (fails on Windows)"
+	^ (SoilJournalFragmentFile path: self path / filename )
+		databaseJournal: self;
+		openReadOnly;
+		yourself
 ]
 
 { #category : #private }
@@ -316,15 +338,20 @@ SoilPersistentDatabaseJournal >> sortedFiles [
 ]
 
 { #category : #accessing }
-SoilPersistentDatabaseJournal >> transactionJournalsStartingAt: index do: aBlock [ 
+SoilPersistentDatabaseJournal >> transactionJournalsStartingAt: index do: aBlock [
 	| fragmentFiles fileIndex |
-	fragmentFiles := self fragmentFiles do: #open.
-	fileIndex := fragmentFiles 
-		detectIndex: [ :fragment | fragment firstTransactionId <= index  ]
-		ifNone: [ self error: 'fileIndex not found, should not happen' ].
-	(fragmentFiles copyFrom: fileIndex to: fragmentFiles size) do: [ :fragmentFile |
-		fragmentFile transactionJournals do: [ :transactionJournal |
-			aBlock value: transactionJournal ] ].
+	"open the fragment files read-only: this only reads them and the journal may
+	hold its current fragment file open for writing. A second open in write mode
+	of the same path fails on Windows. The handles are closed when done"
+	fragmentFiles := self fragmentFiles do: #openReadOnly.
+	[
+		fileIndex := fragmentFiles
+			detectIndex: [ :fragment | fragment firstTransactionId <= index  ]
+			ifNone: [ self error: 'fileIndex not found, should not happen' ].
+		(fragmentFiles copyFrom: fileIndex to: fragmentFiles size) do: [ :fragmentFile |
+			fragmentFile transactionJournals do: [ :transactionJournal |
+				aBlock value: transactionJournal ] ] ]
+		ensure: [ fragmentFiles do: [ :each | each close ] ]
 ]
 
 { #category : #writing }