-
Notifications
You must be signed in to change notification settings - Fork 66
Windows DirectoryWatcher buffer exhaustion recovery workaround. #2149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows DirectoryWatcher buffer exhaustion recovery workaround. #2149
Conversation
c5bfe82
to
0b8d70f
Compare
PR HealthBreaking changes ✔️
Changelog Entry ✔️
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 |
2a0c7f0
to
ee2e3e9
Compare
Add workaround.
ee2e3e9
to
5648491
Compare
Package publishing
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. |
There was a problem hiding this comment.
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.
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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
83adb90
to
1e07325
Compare
1e07325
to
76e5db3
Compare
Updated the workaround, changed the version to I think this is ready now, please take another look @sigurdm :) |
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?