[squeak-dev] Review Request: DoItFirstAfterStartup.1.cs

christoph.thiede at student.hpi.uni-potsdam.de christoph.thiede at student.hpi.uni-potsdam.de
Sun Jan 30 20:02:22 UTC 2022


Hi Marcel, Hi Dave,

I don't see why you would expect this to work. The normal SourceFiles array already contains a writable pointer to the changes file, so of course it cannot be opened writable again. There can only be one writable pointer to a file at one time, at least on Windows. I am more surprised that this rule does not apply on Linux/macOS.

On Windows, apparently we need to close the changes file before re-opening it. We can either implement this manually or apply my proposed patch instead, because even conceptually, I don't see a real sense in making #openSourceFiles itself idempotent. :-)

Dave, you were probably unable to reproduce this in a debugger because once you have replaced the SourceFiles with a read-only pointer and old pointer has been GC'ed, the issue does not occur any longer. There is no limit for parallel read-only pointers on a file.

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2022-01-11T19:12:18-05:00, lewis at mail.msen.com wrote:

> Strange indeed. I tried on a Windows machine, and find that
> "Smalltalk openSourceFiles" raises an incorrect(?) warning about
> read-only changes file. But when I step through it in a debugger,
> everything works correctly and there is no warning.
> 
> I cannot look into it further, but something seems wrong here.
> 
> Dave
> 
> 
> On Tue, Jan 11, 2022 at 10:19:08AM +0100, Marcel Taeumel wrote:
> > Hi Christoph --
> > 
> > This looks strange. Why shouldn't this work on Windows? What's your explanation? Let's not fix the symptom but the cause, please.
> > 
> > -1
> > 
> > Best,
> > Marcel
> > Am 10.01.2022 22:44:42 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:
> > Hi Dave, hi all,
> > 
> > please take a short look at this changeset that I would like to merge into the Trunk in order to fix the DoItTest for Windows. Currently, some of them fail because DoItFirst tries to re-open the source files.
> > 
> > =============== Summary ===============
> > 
> > Change Set:????????????????DoItFirstAfterStartup
> > Date:????????????????????????10 January 2022
> > Author:????????????????????????Christoph Thiede
> > 
> > This changeset fixes the DoItFirst tests on Windows by making sure that no attempt is made to re-open the source fiels while the image has already been started up. This is achieved by extracting FileDirectory startUpDefaultDirectory from FileDirectory startUp and only sending the latter from DoItFirst >> #parse:.
> > 
> > =============== Diff ===============
> > 
> > DoItFirst>>parse: {evaluating} ?? ct 1/10/2022 22:37 (changed)
> > parse: argumentList
> > ????????"Iterate over the argument list, creating actions blocks. Register each action
> > ????????block in the actions dictionary, and collect a list of the actions blocks to be
> > ????????evaluated now. If any action blocks will require files or directory initialization,
> > ????????send the appropriate startUp messages. Answer the list of action blocks to
> > ????????be evaluated."
> > 
> > ????????| args listOfBlocks needsFiles needsDirectory |
> > ????????needsFiles := needsDirectory := false.
> > ????????args := argumentList readStream.
> > ????????listOfBlocks := OrderedCollection new.
> > ????????[ args atEnd ] whileFalse: [ | key |
> > ????????????????(key := self keyFor: args next) caseOf: {
> > ????????????????????????[ #help ] -> [ self addFirst: [ self help ] to: listOfBlocks at: key. needsFiles := true] .
> > ????????????????????????[ #debug ] -> [ self addWithoutEvaluation: [ self debug ] at: key] .
> > ????????????????????????[ #doit ] -> [ | list | list := self nextTokensFrom: args. self add:[ self doIt: list ] to: listOfBlocks at: key. needsFiles := true] .
> > ????????????????????????[ #evaluate ] -> [ | arg | arg := args next. self add:[ self evaluateOption: arg ] to: listOfBlocks at: key. needsFiles := true] .
> > ????????????????????????[ #file ] -> [ | arg | arg := args next. self add:[ self evaluateFileContents: arg ] to: listOfBlocks at: key. needsFiles := true] .
> > ????????????????????????[ #filein ] -> [ | list | list := self nextTokensFrom: args. self add:[ self fileIn: list ] to: listOfBlocks at: key. needsFiles := needsDirectory := true] .
> > ????????????????????????[ #cwd ] -> [ | arg | arg := args next. self addFirst:[ self cwd: arg ] to: listOfBlocks at: key. needsFiles := needsDirectory := true] .
> > ????????????????} otherwise: [] ].
> > ????????needsFiles ifTrue: [ FileStream startUp: true. "initialize stdout and stderr" ].
> > - ????????needsDirectory ifTrue: [ FileDirectory startUp "set default directory" ].
> > + ????????needsDirectory ifTrue: [ FileDirectory startUpDefaultDirectory ].
> > ????????^ listOfBlocks.
> > 
> > 
> > FileDirectory class>>startUp {name utilities} ?? ct 1/10/2022 22:37 (changed)
> > startUp
> > - ????????"Establish the platform-specific FileDirectory subclass. Do any platform-specific startup."
> > - ????????self setDefaultDirectoryClass.
> > 
> > - ????????self setDefaultDirectory: (self dirPathFor: Smalltalk imageName).
> > -
> > - ????????Preferences startInUntrustedDirectory
> > - ????????????????ifTrue:[????????"The SecurityManager may override the default directory to prevent unwanted write access etc."
> > - ????????????????????????????????self setDefaultDirectory: SecurityManager default untrustedUserDirectory.
> > - ????????????????????????????????"Make sure we have a place to go to"
> > - ????????????????????????????????DefaultDirectory assureExistence].
> > + ????????self startUpDefaultDirectory.
> > ????????Smalltalk openSourceFiles.
> > - ????????(Smalltalk classNamed: #DoItFirst) ifNotNil: [ :doit | doit perform: #reevaluateCwd ].
> > 
> > 
> > FileDirectory class>>startUpDefaultDirectory {name utilities} ?? ct 1/10/2022 22:37
> > + startUpDefaultDirectory
> > + ????????"Establish the platform-specific FileDirectory subclass. Do any platform-specific startup."
> > +
> > + ????????self setDefaultDirectoryClass.
> > + ????????self setDefaultDirectory: (self dirPathFor: Smalltalk imageName).
> > +
> > + ????????Preferences startInUntrustedDirectory
> > + ????????????????ifTrue:[????????"The SecurityManager may override the default directory to prevent unwanted write access etc."
> > + ????????????????????????????????self setDefaultDirectory: SecurityManager default untrustedUserDirectory.
> > + ????????????????????????????????"Make sure we have a place to go to"
> > + ????????????????????????????????DefaultDirectory assureExistence].
> > + ????????
> > + ????????(Smalltalk classNamed: #DoItFirst) ifNotNil: [ :doit | doit perform: #reevaluateCwd ].
> > 
> > Best,
> > Christoph
> > 
> > ---
> > Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]
> > ["DoItFirstAfterStartup.1.cs"]
> >
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220130/c7ef1da9/attachment-0001.html>


More information about the Squeak-dev mailing list