Skip to content

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Aug 21, 2025

This is a fun issue dart-lang/build#4154 which looks like an SDK bug dart-lang/sdk#61378 but fortunately I found what seems to be a simple workaround.

The test that's added reliably fails without the fix; it doesn't always fail on the same retry, but 200 retries seems to be plenty. Fortunately it's fast.

And then, with the workaround, it passes :)

It seems reasonably likely that there can be an SDK fix, but the workaround seems useful+safe enough to publish, what do you think please?

Copy link

github-actions bot commented Aug 21, 2025

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
watcher None 1.1.2 1.1.3-wip 1.1.2 ✔️
Changelog Entry ✔️
Package Changed Files

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

Coverage ⚠️
File Coverage
pkgs/watcher/lib/src/directory_watcher.dart 💚 57 %
pkgs/watcher/lib/src/directory_watcher/windows.dart 💔 0 % ⬇️ NaN %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-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
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/bazel_worker/benchmark/benchmark.dart
pkgs/bazel_worker/example/client.dart
pkgs/bazel_worker/example/worker.dart
pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart
pkgs/boolean_selector/example/example.dart
pkgs/clock/lib/clock.dart
pkgs/clock/lib/src/clock.dart
pkgs/clock/lib/src/default.dart
pkgs/clock/lib/src/stopwatch.dart
pkgs/clock/lib/src/utils.dart
pkgs/clock/test/clock_test.dart
pkgs/clock/test/default_test.dart
pkgs/clock/test/stopwatch_test.dart
pkgs/clock/test/utils.dart
pkgs/coverage/lib/src/coverage_options.dart
pkgs/html/example/main.dart
pkgs/html/lib/dom.dart
pkgs/html/lib/dom_parsing.dart
pkgs/html/lib/html_escape.dart
pkgs/html/lib/parser.dart
pkgs/html/lib/src/constants.dart
pkgs/html/lib/src/encoding_parser.dart
pkgs/html/lib/src/html_input_stream.dart
pkgs/html/lib/src/list_proxy.dart
pkgs/html/lib/src/query_selector.dart
pkgs/html/lib/src/token.dart
pkgs/html/lib/src/tokenizer.dart
pkgs/html/lib/src/treebuilder.dart
pkgs/html/lib/src/utils.dart
pkgs/html/test/dom_test.dart
pkgs/html/test/parser_feature_test.dart
pkgs/html/test/parser_test.dart
pkgs/html/test/query_selector_test.dart
pkgs/html/test/selectors/level1_baseline_test.dart
pkgs/html/test/selectors/level1_lib.dart
pkgs/html/test/selectors/selectors.dart
pkgs/html/test/support.dart
pkgs/html/test/tokenizer_test.dart
pkgs/html/test/trie_test.dart
pkgs/html/tool/generate_trie.dart
pkgs/pubspec_parse/test/git_uri_test.dart
pkgs/stack_trace/example/example.dart
pkgs/watcher/test/custom_watcher_factory_test.dart
pkgs/yaml_edit/example/example.dart

@davidmorgan davidmorgan force-pushed the watcher-buffer-exhaustion branch from 2a0c7f0 to ee2e3e9 Compare August 22, 2025 11:43
@davidmorgan davidmorgan force-pushed the watcher-buffer-exhaustion branch from ee2e3e9 to 5648491 Compare August 22, 2025 12:04
Copy link

Package publishing

Package Version Status Publish tag (post-merge)
package:bazel_worker 1.1.3 already published at pub.dev
package:benchmark_harness 2.4.0-wip WIP (no publish necessary)
package:boolean_selector 2.1.2 already published at pub.dev
package:browser_launcher 1.1.3 already published at pub.dev
package:cli_config 0.2.1-wip WIP (no publish necessary)
package:cli_util 0.5.0-wip WIP (no publish necessary)
package:clock 1.1.3-wip WIP (no publish necessary)
package:code_builder 4.10.2-wip WIP (no publish necessary)
package:coverage 1.15.0 already published at pub.dev
package:csslib 1.0.2 already published at pub.dev
package:extension_discovery 2.1.0 already published at pub.dev
package:file 7.0.2-wip WIP (no publish necessary)
package:file_testing 3.1.0-wip WIP (no publish necessary)
package:glob 2.1.3 already published at pub.dev
package:graphs 2.3.3-wip WIP (no publish necessary)
package:html 0.15.6 already published at pub.dev
package:io 1.1.0-wip WIP (no publish necessary)
package:json_rpc_2 4.0.0 already published at pub.dev
package:markdown 7.3.1-wip WIP (no publish necessary)
package:mime 2.0.0 already published at pub.dev
package:oauth2 2.0.4-wip WIP (no publish necessary)
package:package_config 2.3.0-wip WIP (no publish necessary)
package:pool 1.5.2-wip WIP (no publish necessary)
package:process 5.0.5 already published at pub.dev
package:pub_semver 2.2.0 already published at pub.dev
package:pubspec_parse 1.5.0 already published at pub.dev
package:source_map_stack_trace 2.1.3-wip WIP (no publish necessary)
package:source_maps 0.10.14-wip WIP (no publish necessary)
package:source_span 1.10.1 already published at pub.dev
package:sse 4.1.8 already published at pub.dev
package:stack_trace 1.12.1 already published at pub.dev
package:stream_channel 2.1.4 already published at pub.dev
package:stream_transform 2.1.2-wip WIP (no publish necessary)
package:string_scanner 1.4.1 already published at pub.dev
package:term_glyph 1.2.3-wip WIP (no publish necessary)
package:test_reflective_loader 0.3.0 already published at pub.dev
package:timing 1.0.2 already published at pub.dev
package:unified_analytics 8.0.5 already published at pub.dev
package:watcher 1.1.3 ready to publish watcher-v1.1.3
package:yaml 3.1.3 already published at pub.dev
package:yaml_edit 2.2.2 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

error.message.startsWith('Directory watcher closed unexpectedly')) {
unawaited(_watchSubscription?.cancel());
_eventsController.addError(error, stackTrace);
// Wait to work around https://github.com/dart-lang/sdk/issues/61378.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's going on with the reformatting ... this is the only actual change.

@davidmorgan davidmorgan changed the title Watcher buffer exhaustion. Windows DirectoryWatcher buffer exhaustion recovery workaround. Aug 22, 2025
@davidmorgan davidmorgan marked this pull request as ready for review August 22, 2025 12:11
@davidmorgan davidmorgan requested a review from a team as a code owner August 22, 2025 12:11
unawaited(_watchSubscription?.cancel());
_eventsController.addError(error, stackTrace);
// Wait to work around https://github.com/dart-lang/sdk/issues/61378.
await Future<void>.delayed(Duration.zero);
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful if the comment explained the theory for why waiting for zero time helps here. Is the theory here that by letting the event loop run, we (hopefully) drain the queue of pending events from the OS, so it's more likely to be willing to start watching again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discovered that the workaround only improves things, it's still not 100%, so I'll hold off a bit to see if we can learn more :) ... and then update / document.

I think it's actually nothing to do with the OS, it's the OS-interfacing code in the Dart VM that gets into a bad state sometimes. So the delay is something to do with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tweaked the workaround to wait 1ms; this seems to work 100% of the time. I wouldn't bet on it, but I think it's a good enough workaround until the SDK fix arrives.

Added a short explanation and direction to see the issue for more information.

@davidmorgan davidmorgan force-pushed the watcher-buffer-exhaustion branch 2 times, most recently from 83adb90 to 1e07325 Compare August 25, 2025 07:33
@davidmorgan davidmorgan force-pushed the watcher-buffer-exhaustion branch from 1e07325 to 76e5db3 Compare August 25, 2025 07:42
@davidmorgan
Copy link
Contributor Author

Updated the workaround, changed the version to -wip as I think I have another fix to send after this one, and added documentation of how the failure shows / how to handle it on Windows.

I think this is ready now, please take another look @sigurdm :)

@davidmorgan davidmorgan merged commit 53a9f83 into dart-lang:main Aug 25, 2025
18 checks passed
@davidmorgan davidmorgan deleted the watcher-buffer-exhaustion branch August 25, 2025 11:00
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.

3 participants