Fix project file corruption when overwriting a locked file by yashraj4 · Pull Request #3244 · ilastik/ilastik

yashraj4 · 2026-09-13T09:41:11Z

Summary

When trying to create a new project file overwriting an existing file that is locked by another ilastik instance, the file gets corrupted (truncated to 0 bytes) even though the error is caught.

Root cause

h5py.File(path, mode="w") truncates the file at the OS level before acquiring the HDF5 file lock. On Windows, CreateFile with CREATE_ALWAYS truncates the file, then HDF5 tries to lock it. If locking fails (file open in another instance), the OSError is caught in ilastikShell.py, but the file is already corrupted.

Fix

In ProjectManager.createBlankProjectFile(), before opening with mode="w" (which truncates), check if the existing file is accessible by opening it in read mode ("r"). If that fails with OSError (locked by another process), raise immediately without ever attempting the truncating "w" open.

Test

Added test_createBlankProjectFile_does_not_corrupt_locked_file regression test that:

Creates a valid project file
Mocks h5py.File to raise OSError when opened in read mode (simulating a lock)
Verifies createBlankProjectFile raises an error
Verifies the original file is intact (not corrupted)

Before opening with mode='w' (which truncates the file at the OS level), check that the existing file is accessible by opening it in read mode. If it is locked by another process, raise an error immediately without corrupting the file. h5py.File(path, mode='w') truncates the file before acquiring the HDF5 lock, so if the lock fails the file is already corrupted. This pre-check eliminates that window of corruption. Fixes ilastik#3235

codecov · 2026-09-13T10:07:50Z

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 62.53%. Comparing base (5f5e147) to head (f2c0ce6).

Files with missing lines	Patch %	Lines
ilastik/shell/projectManager.py	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3244      +/-   ##
==========================================
+ Coverage   62.51%   62.53%   +0.01%     
==========================================
  Files         537      537              
  Lines       63363    63369       +6     
==========================================
+ Hits        39614    39628      +14     
+ Misses      23749    23741       -8

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

btbest

Hi @yashraj4, thanks for looking into this! In principle I think the fix is the right approach, but as it is the test doesn't actually prove the issue is fixed.

btbest · 2026-09-14T09:22:15Z

+        # If the file already exists, check that it is not locked before attempting to overwrite.
+        # Opening with mode="w" truncates the file at the OS level BEFORE acquiring the HDF5 lock,
+        # so if the file is locked by another process, the file would be corrupted even though
+        # the subsequent OSError is caught. See https://github.com/ilastik/ilastik/issues/3235
+        if os.path.exists(projectFilePath):
+            try:
+                with h5py.File(projectFilePath, mode="r"):
+                    pass
+            except OSError as e:
+                raise OSError(
+                    f"Cannot overwrite project file because it is locked by another process: {projectFilePath}"
+                ) from e
+


comment could be shorter

not sure every OSError would indicate the file is locked. What matters is that it gets re-raised, but the message phrasing should maybe offer locking only as one possible reason

btbest · 2026-09-14T09:26:34Z

    shell._loadProject.assert_not_called()
+
+
+def test_createBlankProjectFile_does_not_corrupt_locked_file():


This test fails on main only due to technicalities (main doesn't raise the specific error string; and writes a different version string in createBlankProjectFile). With those two expectations removed, the test passes fine, even though the corruption issue exists.

It should actually reproduce the file corruption and make sure the fix prevents it.

Also use tmp_path instead of tempfile to stay in-pattern

btbest · 2026-09-14T11:01:31Z

The test works out both simpler and effective at actually testing the bug it's supposed to catch if you have it lock the file with h5py.File in a subprocess, and then call createBlankProjectFile while it's locked from the main process.

Or do you see a specific reason to avoid genuinely reproducing the problem?

yashraj4 · 2026-09-14T16:10:44Z

The test works out both simpler and effective at actually testing the bug it's supposed to catch if you have it lock the file with h5py.File in a subprocess, and then call createBlankProjectFile while it's locked from the main process.

Or do you see a specific reason to avoid genuinely reproducing the problem?

That's a fair point. My concern with a genuine subprocess lock is that file locking behavior is platform-dependent:

Windows: h5py uses mandatory locking (LockFileEx) - the "r" pre-check would fail, which is what we want to test
Linux/macOS: h5py typically uses advisory locking (fcntl.flock) - the "r" pre-check might succeed even while the file is locked by another process, so the bug path wouldn't be exercised the same way

The mock approach tests the specific invariant we care about: that mode="w" is never attempted when mode="r" fails. But I agree it's less convincing than a real scenario.

Would you be okay with keeping the mock-based test as a unit test and relying on the existing manual reproduction steps (from the issue) for integration validation? Or would you prefer I attempt a cross-platform subprocess approach with platform-specific lock detection?

btbest · 2026-09-14T16:30:12Z

Sure, maybe there are platform-specific behaviours, but all the test needs to do is replicate the relevant production situation: process 1 has used openProjectFile to lock the file, and process 2 attempts to overwrite it with createBlankProjectFile. You don't need to interact with file-locking behaviours at all in the test, because they are simply whatever locking behaviours happen within the actual functions ilastik uses.

If the corruption issue is specific to Windows, then the test will currently only fail on main for Windows, but the test correctly expresses that we need to not corrupt the file on any platform when this situation occurs.

…corruption prevention - Shortened the pre-check comment - Changed error message to not assume locking is the only reason - Rewrote test to prove that mode='w' is never called when mode='r' fails, which is the actual mechanism that prevents file corruption - Used tmp_path fixture instead of tempfile.TemporaryDirectory()

btbest requested changes Sep 14, 2026

View reviewed changes

yashraj4 force-pushed the fix/project-file-corruption-on-locked-overwrite branch from f2c0ce6 to b9067bf Compare September 14, 2026 17:53

Navigation Menu

Repository navigation

Fix project file corruption when overwriting a locked file - #3244

Fix project file corruption when overwriting a locked file#3244
yashraj4 wants to merge 2 commits into
ilastik:mainfrom
yashraj4:fix/project-file-corruption-on-locked-overwrite

yashraj4 commented Sep 13, 2026

Uh oh!

codecov Bot commented Sep 13, 2026 •

edited

Loading

Uh oh!

btbest left a comment

Uh oh!

btbest Sep 14, 2026

Uh oh!

btbest Sep 14, 2026

Uh oh!

btbest commented Sep 14, 2026

Uh oh!

yashraj4 commented Sep 14, 2026

Uh oh!

btbest commented Sep 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		shell._loadProject.assert_not_called()


		def test_createBlankProjectFile_does_not_corrupt_locked_file():

Conversation

yashraj4 commented Sep 13, 2026

Summary

Root cause

Fix

Test

Uh oh!

codecov Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

btbest left a comment

Choose a reason for hiding this comment

Uh oh!

btbest Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

btbest Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

btbest commented Sep 14, 2026

Uh oh!

yashraj4 commented Sep 14, 2026

Uh oh!

btbest commented Sep 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 13, 2026 •

edited

Loading