diff --git a/host-setup/bootstrap.ps1 b/host-setup/bootstrap.ps1 index d9afb285..71b722e5 100644 --- a/host-setup/bootstrap.ps1 +++ b/host-setup/bootstrap.ps1 @@ -86,7 +86,6 @@ $MODE = '' $RESOLVED = '' $TREE = '' $PWSH_PATH = '' -# Held rather than disposed, since the lock it carries lasts until this process ends. $LOCK = $null function usage { @@ -286,6 +285,14 @@ function Lock-Directory { } } +# A run typed at an interactive prompt runs in the caller's process. +# A lock left for that process to release outlives the run and refuses the next one in the same session. +function Unlock-Directory { + if (-not $script:LOCK) { return } + $script:LOCK.Dispose() + $script:LOCK = $null +} + # Refuses to remove a tree this run did not create, rather than trusting the name. function Remove-Owned { param([string]$Path) @@ -651,7 +658,8 @@ function main { 'dev' { Invoke-StandUp -Kind 'dev' } } } finally { - Invoke-Cleanup + # Released after the cleanup, so the cleanup still runs under the lock, and on die's exit too, which runs this finally on its way out. + try { Invoke-Cleanup } finally { Unlock-Directory } } step 'Done' diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 859a12ee..ac1fe7f4 100755 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -932,7 +932,7 @@ def test_the_directory_the_skills_installer_registers_outlives_the_run(self) -> 'Test-KeepsTree', 'Get-TreeName', 'Get-TreePath', 'Get-StagingPath', 'Get-RetiredPath', 'Get-ArchivePath', 'Get-LockPath', 'Lock-Directory', 'Test-Ownership', 'Remove-Owned', 'Get-Tree', 'Remove-Tree', 'Move-Tree', 'Invoke-SwapIn', 'Invoke-Cleanup', 'Resolve-Directory', 'Invoke-Tool', 'Invoke-SkillsInstall', - 'Show-Report', 'Test-Elevated', 'Test-AgentsUser', 'Invoke-Upgrade', 'Invoke-AgentsInstall' + 'Show-Report', 'Test-Elevated', 'Test-AgentsUser', 'Invoke-Upgrade', 'Invoke-AgentsInstall', 'Unlock-Directory', 'main' ) foreach ($definition in $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $wanted -contains $node.Name }, $true)) { . ([scriptblock]::Create($definition.Extent.Text)) @@ -1326,6 +1326,70 @@ def test_the_directory_lock_refuses_a_second_run_and_frees_when_the_first_ends(s result = self.run_loader("Lock-Directory") self.assertEqual(result.returncode, 0, result.stderr) + def test_a_run_ending_inside_its_callers_process_releases_the_directory_lock(self) -> None: + """Two runs in one pwsh process, as two typed at one interactive prompt are, the first ending through `die`'s exit. + + The exit ends only the script it is in, so the process and anything the run left open outlive it. + """ + with tempfile.TemporaryDirectory() as directory: + work = Path(directory) + harness = work / "harness.ps1" + harness.write_text(POWERSHELL_TREE_HARNESS, encoding="utf-8") + first = work / "first.ps1" + first.write_text( + "$script:WANT_HELP = $false\n" + "function Test-Prerequisite {}\n" + "function Resolve-Mode { 'report' }\n" + "function Resolve-Ref {}\n" + "function Get-Tree { die 'the download failed' }\n" + "main", + encoding="utf-8", + ) + second = work / "second.ps1" + # The same mode as the first run's, since the mode names the lock file and the harness defaults to another. + second.write_text( + "$script:MODE = 'report'\nLock-Directory\n[Console]::Out.WriteLine('locked')", + encoding="utf-8", + ) + driver = work / "driver.ps1" + driver.write_text( + "param([string]$Harness, [string]$Loader, [string]$Dir, [string]$First, [string]$Second)\n" + "& $Harness -Loader $Loader -Dir $Dir -BodyFile $First\n" + '[Console]::Out.WriteLine("first exited $LASTEXITCODE")\n' + "$global:LASTEXITCODE = 0\n" + "& $Harness -Loader $Loader -Dir $Dir -BodyFile $Second\n" + "exit $LASTEXITCODE\n", + encoding="utf-8", + ) + result = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(driver), + "-Harness", + str(harness), + "-Loader", + str(BOOTSTRAP_PS), + "-Dir", + str(self.dir), + "-First", + str(first), + "-Second", + str(second), + ], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=120, + ) + self.assertIn("the download failed", result.stderr) + self.assertIn("first exited 1", result.stdout) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("locked", result.stdout) + @unittest.skipUnless(sys.platform == "linux", "the lock bootstrap.sh takes is a Linux flock") def test_a_lock_bootstrap_sh_holds_refuses_this_loader(self) -> None: """FileShare.None is a flock on Linux, which is the one thing that makes the two loaders' locks one lock."""