Skip to content

[hooks_runner] Write recorded_uses.json under directory lock in link() - #3770

Merged
goderbauer merged 2 commits into
dart-lang:mainfrom
goderbauer:fix-recorded-uses-lock-race
Nov 3, 2026
Merged

goderbauer merged 2 commits into
dart-lang:mainfrom
goderbauer:fix-recorded-uses-lock-race

Conversation

@goderbauer

@goderbauer goderbauer commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Previously, NativeAssetsBuildRunner.link() wrote recorded_uses.json into buildDirUri before acquiring runUnderDirectoriesLock in _runHookForPackageCached. Concurrent link() runs sharing the same buildDirUri could truncate or overwrite recorded_uses.json while another link hook or dependency hash check was reading it, causing either silent wrong tree-shaking outputs or 'File modified during build. Build must be rerun.' errors.

Move the creation and writing of recorded_uses.json inside the runUnderDirectoriesLock critical section in _runHookForPackageCached, prior to recording lastModifiedCutoffTime and checking dependenciesHashes.findOutdatedDependency.

Add a regression test in resources_test.dart covering concurrent link() invocations with identical and distinct recordings.

Fixes #3769 (discovered during review of https://dart-review.googlesource.com/c/sdk/+/570263) with Gemini's assistance.

@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3770   +/-   ##
=======================================
  Coverage   87.94%   87.94%           
=======================================
  Files         310      310           
  Lines       19848    19848           
=======================================
  Hits        17456    17456           
  Misses       2392     2392           
Flag Coverage Δ
native_pkgs_macos 86.47% <100.00%> (ø)
native_pkgs_ubuntu 72.14% <100.00%> (ø)
native_pkgs_windows 75.47% <100.00%> (ø)

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%> (ø)
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 Δ
...ooks_runner/lib/src/build_runner/build_runner.dart 90.95% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Previously, NativeAssetsBuildRunner.link() wrote recorded_uses.json into buildDirUri before acquiring runUnderDirectoriesLock in _runHookForPackageCached. Concurrent link() runs sharing the same buildDirUri could truncate or overwrite recorded_uses.json while another link hook or dependency hash check was reading it, causing either silent wrong tree-shaking outputs or 'File modified during build' errors.

Move the creation and writing of recorded_uses.json inside the runUnderDirectoriesLock critical section in _runHookForPackageCached, prior to recording lastModifiedCutoffTime and checking dependenciesHashes.findOutdatedDependency.

Add a regression test in resources_test.dart covering concurrent link() invocations with identical and distinct recordings.
@goderbauer
goderbauer force-pushed the fix-recorded-uses-lock-race branch from cbe01e5 to 74085b0 Compare October 28, 2026 10:36
@goderbauer

Copy link
Copy Markdown
Contributor Author

CI failures are unrelated and will be fixed by #3771.

@goderbauer
goderbauer requested a review from dcharkes October 28, 2026 10:50

@dcharkes dcharkes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @goderbauer !

@goderbauer
goderbauer merged commit d294d99 into dart-lang:main Nov 3, 2026
53 of 54 checks passed
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Nov 5, 2026
Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/24e09c4..36f57a9):
  36f57a99  Tue Nov 3 14:26:17 2026 +0100  Sigurd Meldgaard  [os_detect] Remove bin/os_detect.dart (dart-lang/core#1015)
  86bacfca  Tue Nov 3 05:15:19 2026 -0500  Justin White  [characters] Fix RangeError on an unpaired tail surrogate at range start (dart-lang/core#1033)

dartdoc (https://github.com/dart-lang/dartdoc/compare/db94437..2f6847b):
  2f6847bd  Thu Oct 29 11:47:04 2026 +0200  Sergey G. Grekhov  Add tests for invalid doc comments placement (dart-lang/dartdoc#4301)
  17f2f0a3  Thu Oct 29 10:16:36 2026 +0100  Sarah Zakarias  Compare base elements and unify properties in doc-reference-target-differs (dart-lang/dartdoc#4307)

dartdoc is held back at 2f6847b, before dart-lang/dartdoc#4312, because its
mock SDK version changes conflict in the google3 3-way merge with the hotfix
in cl/1005963642.

ecosystem (https://github.com/dart-lang/ecosystem/compare/f8faabf..54e1673):
  54e1673  Wed Nov 4 09:46:54 2026 +0100  Sigurd Meldgaard  fix(firehose): resolve dartExecutable via package:cli_util (dart-lang/ecosystem#469)
  d754065  Tue Nov 3 10:09:45 2026 -0800  Kevin Moore  refactor(corpus): use path.join and seal closed type hierarchies (dart-lang/ecosystem#476)

http (https://github.com/dart-lang/http/compare/8c0d66a..1f869d0):
  1f869d0  Tue Nov 3 15:51:52 2026 -0800  dependabot[bot]  chore(deps): bump the github-actions group with 8 updates (dart-lang/http#2052)
  6d7f402  Tue Nov 3 14:12:04 2026 -0800  Brian Quinlan  feat(cupertino_http): support custom trust eval (dart-lang/http#2038)
  73f8c36  Tue Nov 3 10:15:16 2026 -0800  Brian Quinlan  chore(cupertino_http): ready 3.1.1 release (dart-lang/http#2056)

native (https://github.com/dart-lang/native/compare/b442038..1838637):
  183863758  Wed Nov 4 09:01:27 2026 -0300  Daco Harkes  [hooks_runner] Add `PKG_CONFIG_PATH` to environment variables allowlist (dart-lang/native#3779)
  c9e8118d4  Wed Nov 4 08:16:20 2026 +1100  Liam Appelbe  [jni] Fix release mode segfault (dart-lang/native#3777)
  5999cf7e4  Wed Nov 4 08:16:16 2026 +1100  Liam Appelbe  [objective_c] Prepare to publish (dart-lang/native#3778)
  d294d9942  Tue Nov 3 15:15:49 2026 +0100  Michael Goderbauer  [hooks_runner] Write recorded_uses.json under directory lock in link() (dart-lang/native#3770)
  33aa5ef04  Tue Nov 3 11:42:38 2026 +1100  Liam Appelbe  [objective_c] Cheaper and safer object/block validity checks (dart-lang/native#3776)

protobuf (https://github.com/dart-lang/protobuf/compare/1d52301..19191c8):
  19191c8  Wed Nov 4 17:14:33 2026 +0100  Sigurd Meldgaard  Use dart.tool.dart2wasm in json.dart so DDC uses json_web.dart (google/protobuf.dart#1108)

tools (https://github.com/dart-lang/tools/compare/1579f8d..a342744):
  a3427446f  Wed Nov 4 15:11:35 2026 +0100  Jonas Finnemann Jensen  Update CODEOWNERS for pkgs/markdown (dart-lang/tools#2795)
  08c9afdc0  Wed Nov 4 14:40:44 2026 +0100  Jonas Finnemann Jensen  [markdown] Check URL boundary before matching in AutolinkExtensionSyntax (dart-lang/tools#2786)
  0fc5edcae  Wed Nov 4 14:35:48 2026 +0100  Jonas Finnemann Jensen  [markdown] Skip unclosed inline HTML tags before matching RegExp (dart-lang/tools#2788)
  b1f6f9956  Wed Nov 4 14:31:55 2026 +0100  Jonas Finnemann Jensen  [markdown] Defer shortcut reference link label extraction in LinkSyntax.close (dart-lang/tools#2785)
  ab135e072  Wed Nov 4 14:30:37 2026 +0100  Jonas Finnemann Jensen  [markdown] Avoid quadratic trailing-paren scan in AutolinkExtensionSyntax (dart-lang/tools#2787)
  f329bf3ea  Wed Nov 4 17:09:34 2026 +0530  Ananya Kaul  [code_builder] Separate chained cascades after nullable casts (dart-lang/tools#2704)
  0a57e3446  Wed Nov 4 05:28:40 2026 +0530  Aman Maurya  fix(coverage): allow empty coverage files from skipped test suites (`#2667`) (dart-lang/tools#2668)
  e95958f6a  Tue Nov 3 21:53:57 2026 +0000  Keerti Parthasarathy  [unified_analytics] Add root_tool and leaf_tool to every event; key consent on root_tool (dart-lang/tools#2784)
  5da06a508  Tue Nov 3 13:11:04 2026 +0100  Jonas Finnemann Jensen  [markdown] Reject unescaped '(' in parenthesized inline link titles (dart-lang/tools#2770)
  f3a5c9277  Tue Nov 3 13:09:32 2026 +0100  Jonas Finnemann Jensen  [markdown] Cap parenthesis nesting in bare inline link destinations (dart-lang/tools#2769)
  820ce5fb7  Tue Nov 3 13:04:35 2026 +0100  Jonas Finnemann Jensen  [markdown] Reject unescaped '(' in parenthesized link reference titles (dart-lang/tools#2771)
  775c5d080  Tue Nov 3 13:03:43 2026 +0100  Jonas Finnemann Jensen  [markdown] Require balanced parentheses before bare inline link titles (dart-lang/tools#2768)
  5ebd52bc5  Tue Nov 3 13:03:08 2026 +0100  Jonas Finnemann Jensen  [markdown] Reject unescaped '<' in bracketed inline link destinations (dart-lang/tools#2767)
  feb7ea928  Tue Nov 3 13:02:15 2026 +0100  Jonas Finnemann Jensen  [markdown] Fix 999-character limit in LinkParser.parseLabel (dart-lang/tools#2766)
  31888a5da  Tue Nov 3 13:01:43 2026 +0100  Jonas Finnemann Jensen  [markdown] Enforce 999-character limit in _parseReferenceLinkLabel (dart-lang/tools#2765)
  e016c5d70  Tue Nov 3 09:26:32 2026 +0100  dart-ecosystem-bot[bot]  [boring] Pin the prebuilt binaries of boring-binaries-20261101-f869fdca423c (dart-lang/tools#2778)
  8c7935573  Mon Nov 2 10:15:24 2026 -0800  Kevin Moore  [html] Move WPT selector fixtures to third_party/wpt-selectors (dart-lang/tools#2772)

web (https://github.com/dart-lang/web/compare/45166fd..f388bf5):
  f388bf5  Sun Nov 1 06:57:22 2026 +0000  dependabot[bot]  Bump the github-actions group with 4 updates (dart-lang/web#611)
  658ce2f  Sat Oct 31 12:47:20 2026 +0100  Moritz  Migrate to ecosystem health publish check and setup-dart publish workflow (dart-lang/web#610)

Change-Id: Ifcae02c4712b4b27a5a8ab1c57d8dcf13a6700cf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/574281
Reviewed-by: Brian Quinlan <[email protected]>
Commit-Queue: Nate Bosch <[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] Race condition in NativeAssetsBuildRunner.link: recorded_uses.json is written outside runUnderDirectoriesLock

2 participants