Skip to content

[hooks_runner] Fail eagerly when file locking is not supported on file system - #3679

Merged
auto-submit[bot] merged 3 commits into
mainfrom
locking-unsupported-fail-eagerly
Oct 13, 2026
Merged

auto-submit[bot] merged 3 commits into
mainfrom
locking-unsupported-fail-eagerly

Conversation

@dcharkes

@dcharkes dcharkes commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Instead of treating all FileSystemExceptions during RandomAccessFile.lock as lock contention and waiting until timeout, check the OS error code against the host OS contention error codes (EAGAIN/EACCES on Linux and macOS, ERROR_SHARING_VIOLATION/ERROR_LOCK_VIOLATION on Windows) and fail eagerly with a descriptive message when process locks are not supported.

Closes: #3651

Also, actually close the file.
(Sorry diff is unreadable in github UI, see in vscode, everything is nested 1 level deeper)

@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
hooks_runner Breaking 1.6.4 2.0.0-wip 2.0.0-wip ✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check (for example, for non-user-visible changes when the package version is already at -wip).

@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.94%. Comparing base (cc7e4b0) to head (c0b3ef7).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3679      +/-   ##
==========================================
+ Coverage   87.90%   87.94%   +0.04%     
==========================================
  Files         310      310              
  Lines       19841    19848       +7     
==========================================
+ Hits        17441    17456      +15     
+ Misses       2400     2392       -8     
Flag Coverage Δ
ffigen 91.36% <ø> (ø)
native_pkgs_macos 86.47% <100.00%> (+0.11%) ⬆️
native_pkgs_ubuntu 72.14% <100.00%> (+0.12%) ⬆️
native_pkgs_windows 75.47% <100.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
code_assets 94.96% <ø> (ø)
data_assets 91.56% <ø> (ø)
ffi 100.00% <ø> (ø)
ffigen 91.45% <ø> (ø)
hooks 86.40% <ø> (ø)
hooks_runner 90.52% <100.00%> (+0.80%) ⬆️
jni 66.34% <ø> (ø)
jni_flutter ∅ <ø> (∅)
jni_util 25.00% <ø> (ø)
jnigen 82.85% <ø> (ø)
json_syntax_generator ∅ <ø> (∅)
native_test_helpers 85.71% <ø> (ø)
native_toolchain_c 95.97% <ø> (ø)
objective_c 79.53% <ø> (ø)
pub_formats ∅ <ø> (∅)
record_use 81.48% <ø> (ø)
snippet_tool 66.23% <ø> (ø)
swift2objc 92.95% <ø> (ø)
swiftgen 80.41% <ø> (ø)
test_case_selector 97.28% <ø> (ø)
Files with missing lines Coverage Δ
pkgs/hooks_runner/lib/src/locking/locking.dart 100.00% <100.00%> (+23.52%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dcharkes
dcharkes marked this pull request as ready for review September 23, 2026 11:02
'process locks are not supported is not supported. '
'Please move your project to a different location.';
logger?.severe(message);
throw FileSystemException(message, file.path, e.osError);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also fired when writeString or unlock fail with a FileSystemException (which I believe they can). An error message about lock acquisition is confusing in those cases. Should this error handling be more closely around just the lock call?

if (errorCode != null && !contentionErrorCodes.contains(errorCode)) {
final message =
'Could not acquire the lock to ${file.path}. '
'Running hooks_runner on a project on a file system where '

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do people that are gonna see this message know what hooks_runner is? Isn't that more of an implementation detail of the flutter and dart cli that it uses hooks_runner?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, should we consider error codes or a list of known exceptions that FlutterTool and Dartdev can catch and give a better error message?

@dcharkes

Copy link
Copy Markdown
Collaborator Author

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…e system

Instead of treating all `FileSystemException`s during `RandomAccessFile.lock` as lock contention and waiting until timeout, check the OS error code against the host OS contention error codes (`EAGAIN`/`EACCES` on Linux and macOS, `ERROR_SHARING_VIOLATION`/`ERROR_LOCK_VIOLATION` on Windows) and fail eagerly with a descriptive message when process locks are not supported.

Closes: #3651
@dcharkes
dcharkes force-pushed the locking-unsupported-fail-eagerly branch from a21c25a to c0b3ef7 Compare October 13, 2026 12:26
@auto-submit
auto-submit Bot merged commit 3cbe319 into main Oct 13, 2026
64 checks passed
@auto-submit
auto-submit Bot deleted the locking-unsupported-fail-eagerly branch October 13, 2026 13:47
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 20, 2026
…, web

Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/4d80610..72e71ca):
  72e71ca0  Mon Oct 19 21:57:11 2026 +0200  Jonas Finnemann Jensen  Optimize `HashSink` and `_MD5Sink.updateHash` (RFC impl) (dart-lang/core#1013)

dartdoc (https://github.com/dart-lang/dartdoc/compare/e1859cf..c0e19cf):
  c0e19cf6  Mon Oct 19 11:41:42 2026 -0700  Konstantin Scheglov  Import analyzer ast with explicit show. (dart-lang/dartdoc#4300)

ecosystem (https://github.com/dart-lang/ecosystem/compare/622ef61..0ab0cfd):
  0ab0cfd  Mon Oct 19 15:44:26 2026 +0200  Moritz  fix(firehose): bump dependency_validator to support analyzer 14 (dart-lang/ecosystem#453)
  44a0768  Wed Oct 14 09:37:29 2026 -0700  Nate Bosch  Improve firehose publish validation log output (dart-lang/ecosystem#468)

http (https://github.com/dart-lang/http/compare/7d7aae2..a8a24a0):
  a8a24a0  Mon Oct 19 13:31:34 2026 -0700  Kevin Moore  docs(web_socket): remove non-user-visible changelog entry (dart-lang/http#2023)
  6f6ac5f  Mon Oct 19 16:29:43 2026 -0400  Mouad Debbar  [http_multi_server] Retry `loopback(0)` when IPv6 port is already bound (dart-lang/http#2025)
  6e91ce3  Tue Oct 13 16:25:39 2026 -0700  Kevin Moore  Sync root README.md package table from pubspec.yaml files (dart-lang/http#2024)

native (https://github.com/dart-lang/native/compare/cc7e4b0..3cbe319):
  3cbe319e3  Tue Oct 13 15:47:18 2026 +0200  Daco Harkes  [hooks_runner] Fail eagerly when file locking is not supported on file system (dart-lang/native#3679)

shelf (https://github.com/dart-lang/shelf/compare/b2cb503..b66665b):
  b66665b  Fri Oct 16 15:12:05 2026 -0700  Kevin Moore  chore: add health workflow and prepare shelf 1.4.3, shelf_router 1.1.5, and shelf_static 1.2.0 (dart-lang/shelf#546)

test (https://github.com/dart-lang/test/compare/9bf69a8..d1afeb1):
  d1afeb14  Fri Oct 16 20:41:51 2026 -0400  Mouad Debbar  Confirmation that `tearDown` is called on timeout (dart-lang/test#2798)
  3f6c2f14  Thu Oct 15 18:21:35 2026 -0700  Jacob MacDonald  Enable asserts in the cli compiler (dart-lang/test#2795)

tools (https://github.com/dart-lang/tools/compare/c1471db..f6c7a2d):
  f6c7a2d62  Mon Oct 19 07:59:24 2026 -0700  Kevin Moore  [pubspec_parse] Preserve input maps and YamlMap source spans (dart-lang/tools#2653)
  f1a55d953  Mon Oct 19 07:53:51 2026 -0700  Kevin Moore  [yaml] Fix tag URI decoding, numeric tag RangeErrors, and dead checks (dart-lang/tools#2630)
  340ded284  Mon Oct 19 07:43:31 2026 -0700  Kevin Moore  [unified_analytics] Prepare for 8.2.0 release (dart-lang/tools#2689)
  a73703543  Fri Oct 16 11:55:03 2026 -0700  Kevin Moore  [unified_analytics] Bump version to 8.2.0-wip (dart-lang/tools#2688)

web (https://github.com/dart-lang/web/compare/63e6662..4e60e61):
  4e60e61  Mon Oct 19 10:02:22 2026 -0700  dependabot[bot]  Bump source-map-js from 1.2.1 to 1.2.2 in /web_generator (dart-lang/web#591)
  7d63637  Mon Oct 19 09:50:26 2026 -0700  Kevin Moore  Add an api.txt golden for package:web (dart-lang/web#590)

[email protected]

Change-Id: Ibe6ac8a8246a27caa8a4846ea1acc78c801ee2ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/566781
Commit-Queue: Nate Bosch <[email protected]>
Reviewed-by: Brian Quinlan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hooks_runner] TimeoutException when file locking is unsupported by the filesystem (e.g. SMB shares on macOS)

2 participants