fix(pool): fix forEach cleanup on error and enable WasmGC tests by kevmoo · Pull Request #2360 · dart-lang/tools

kevmoo · 2026-04-04T23:24:29Z

Update .github/workflows/pool.yaml to use Node.js v22, enabling WasmGC support for WebAssembly tests.
Consolidate browser and node tests to include the dart2wasm compiler.
Refactor Pool.forEach to use dual futures: one for eager error propagation and another to wait for all workers to complete before closing the stream.
This ensures onCancel in forEach waits for all active workers to release their resources even on early failure.
Nullify resumeCompleter in onCancel to prevent potential double-completion.
Add a test verifying that cancel waits for all workers to finish on error.
Added an example

- Add tests for edge cases in PoolResource and Pool.close(). - Improve internal type safety and readability in Pool implementation. - Update version to 1.5.3-wip. Fix forEach edge cases and apply optimizations - Complete resumeCompleter on cancel to prevent hangs when paused. - Stop workers early on fatal errors in forEach. - Avoid pulling items from iterator after cancellation. - Remove invalid assertion in onCancel triggered after errors. - Use onError instead of catchError for async handling. - Add tests for cancel while paused and iterator error behavior. Tests now pass on node,chrome x dart2js,dart2wasm

github-actions · 2026-04-04T23:25:08Z

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:bazel_worker	1.1.5	already published at pub.dev
package:benchmark_harness	2.4.0	already published at pub.dev
package:boolean_selector	2.1.2	already published at pub.dev
package:browser_launcher	1.2.0-wip	WIP (no publish necessary)
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.11.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.4.0-wip	WIP (no publish necessary)
package:html	0.15.7-wip	WIP (no publish necessary)
package:io	1.1.0-wip	WIP (no publish necessary)
package:json_rpc_2	4.1.0	already published at pub.dev
package:markdown	7.3.1	already published at pub.dev
package:mime	2.1.0-wip	WIP (no publish necessary)
package:oauth2	2.0.5	already published at pub.dev
package:package_config	2.3.0-wip	WIP (no publish necessary)
package:pool	1.5.3	ready to publish	`pool-v1.5.3`
package:process	5.0.5	(error) pubspec version (5.0.5) and changelog (5.0.6-wip) don't agree
package:pub_semver	2.2.0	already published at pub.dev
package:pubspec_parse	1.6.0-wip	WIP (no publish necessary)
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.2	already published at pub.dev
package:sse	4.2.0-wip	(error) pubspec version (4.2.0-wip) and changelog (4.2.0) don't agree
package:stack_trace	1.12.2-wip	(error) pubspec version (1.12.2-wip) and changelog (1.12.2-dev) don't agree
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.2-wip	WIP (no publish necessary)
package:term_glyph	1.2.3-wip	WIP (no publish necessary)
package:test_reflective_loader	0.6.0	ready to publish	`test_reflective_loader-v0.6.0`
package:timing	1.0.2	already published at pub.dev
package:unified_analytics	8.0.14	already published at pub.dev
package:watcher	1.2.2-wip	WIP (no publish necessary)
package:yaml	3.1.3	already published at pub.dev
package:yaml_edit	2.2.4	already published at pub.dev

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

github-actions · 2026-04-04T23:25:37Z

PR Health

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.

Coverage

⚠️

File	Coverage
pkgs/pool/example/example.dart	💔 Not covered
pkgs/pool/lib/pool.dart	💚 100 % ⬆️ 3 %

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.

Unused Dependencies ✔️

Package	Status
pool	✔️ All dependencies utilized correctly.

For details on how to fix these, see dependency_validator.

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

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
pool	None	1.5.2	1.5.3	1.5.2	✔️

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.

License Headers ✔️

// Copyright (c) 2026, 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/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/watcher/test/custom_watcher_factory_test.dart

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

gemini-code-assist

Code Review

This pull request updates the pool package to version 1.5.3-wip, introducing support for asynchronous release callbacks and improving the forEach method's handling of cancellation and iterator errors. Key changes include adding a cancellation check in the worker loop and ensuring that errors in the doneFuture trigger a cancellation of pending tasks. New tests have been added to verify resource release states, multiple close calls, and early stream termination. Feedback highlights a potential deadlock in the onCancel handler when awaiting doneFuture with a synchronous StreamController and suggests nullifying resumeCompleter after completion to prevent potential side effects.

kevmoo · 2026-04-05T00:29:01Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces several improvements to the pool package, including support for asynchronous release callbacks, enhanced error handling during resource processing, and better cancellation logic for streams. Additionally, it adds comprehensive tests to verify these new behaviors and edge cases. I have no feedback to provide as the changes are well-implemented and appropriately tested.

kevmoo · 2026-04-05T16:31:08Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the pool package to version 1.5.3-wip, adding a usage example and significantly improving the robustness of Pool.forEach. Key changes include ensuring all workers complete before the stream closes—even during errors or cancellation—and refactoring internal resource release callbacks to support FutureOr. Feedback was provided to expand the changelog to accurately reflect these significant bug fixes and internal improvements.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

natebosch

We shouldn't mix cleanup with bug fixes.

Which of these tests are related to the bug fix?

kevmoo · 2026-04-07T02:26:41Z

    Future.sync(onRelease).then((value) {
      _onReleaseCompleters.removeFirst().complete(PoolResource._(this));
-    }).catchError((Object error, StackTrace stackTrace) {
+    }).onError((Object error, StackTrace stackTrace) {


move to the better typed version!

kevmoo · 2026-04-07T02:44:14Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the pool package to version 1.5.3-wip, primarily fixing Pool.forEach to ensure all workers complete before the stream is closed during errors or cancellations. It also adds a usage example and expands the test suite to cover resource release failures and multiple close calls. A critical resource leak was identified in _runOnRelease where a failed onRelease callback prevents the resource slot from being freed, potentially leading to a deadlock.

kevmoo · 2026-04-07T05:59:07Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the pool package to version 1.5.3-wip, primarily fixing Pool.forEach to ensure all workers complete before the stream is closed, even in scenarios involving errors or cancellation. Additionally, it improves resource management by ensuring slots are released if allowRelease callbacks fail and adds a new example file along with extensive tests for these behaviors. I have no feedback to provide as there were no review comments to evaluate.

kevmoo · 2026-04-07T06:06:43Z

We shouldn't mix cleanup with bug fixes.

Which of these tests are related to the bug fix?

The only cleanup was adding the example - want me to pull that out?

I figured we might as well publish, too!

natebosch · 2026-04-07T18:23:56Z

The only cleanup was adding the example

The tests have too many new cases and other edited lines to be strictly testing the change.

kevmoo · 2026-04-07T18:26:00Z

          await subscription.cancel();

-          expect(eventCount, 1 + dataSize ~/ 2);
+          // Because workers run in parallel, they might produce extra items in


This is the only unrelated changed to the forEach changes - just allows node tests to work @natebosch !

kevmoo · 2026-04-07T18:26:56Z

The only cleanup was adding the example

The tests have too many new cases and other edited lines to be strictly testing the change.

Part of this work was creating many more test cases to validate edge cases. I can remove the tests not related to the fix, if you prefer.

natebosch · 2026-04-07T19:01:45Z

I can remove the tests not related to the fix, if you prefer.

Yes, it would be significantly easier to review a behavior change + tests independently from tests for behavior that is not changing. I didn't look closely at the tests yet, but test cases like cancel while paused completes look unrelated.

kevmoo added 2 commits April 4, 2026 16:22

fun

7e6292e

github-actions Bot added type-infra A repository infrastructure change or enhancement package:pool labels Apr 4, 2026

gemini-code-assist Bot reviewed Apr 4, 2026

View reviewed changes

Comment thread pkgs/pool/lib/pool.dart

Comment thread pkgs/pool/lib/pool.dart Outdated

kevmoo added 2 commits April 4, 2026 16:33

enable node!

9631a44

two more fixes

d9d01a4

gemini-code-assist Bot reviewed Apr 5, 2026

View reviewed changes

kevmoo changed the title ~~pool fun~~ fix(pool): fix forEach cleanup on error and enable WasmGC tests Apr 5, 2026

kevmoo marked this pull request as ready for review April 5, 2026 03:11

kevmoo requested a review from a team as a code owner April 5, 2026 03:11

kevmoo added 3 commits April 4, 2026 20:12

private bits in test

e014f80

add an example while we're at it!

9d7131c

tiny nit about iterable silly

00d95e7

gemini-code-assist Bot reviewed Apr 5, 2026

View reviewed changes

Comment thread pkgs/pool/CHANGELOG.md

Update pkgs/pool/CHANGELOG.md

edcee46

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

natebosch requested changes Apr 7, 2026

View reviewed changes

Comment thread pkgs/pool/example/example.dart

Comment thread pkgs/pool/lib/pool.dart Outdated

Comment thread pkgs/pool/lib/pool.dart Outdated

kevmoo commented Apr 7, 2026

View reviewed changes

kevmoo and others added 3 commits April 6, 2026 19:35

review feedback

be69385

Merge branch 'main' into pool_fun

744f5b8

Merge remote-tracking branch 'origin/pool_fun' into pool_fun

b325bbc

gemini-code-assist Bot reviewed Apr 7, 2026

View reviewed changes

Comment thread pkgs/pool/lib/pool.dart

a test and a fix

f37d2e0

gemini-code-assist Bot reviewed Apr 7, 2026

View reviewed changes

prepare release

89eaa62

rollback other changes

5def979

kevmoo commented Apr 7, 2026

View reviewed changes

Conversation

kevmoo commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package publishing

Uh oh!

github-actions Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

kevmoo commented Apr 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

kevmoo commented Apr 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

natebosch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevmoo Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Apr 7, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

kevmoo commented Apr 7, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

kevmoo commented Apr 7, 2026

Uh oh!

natebosch commented Apr 7, 2026

Uh oh!

kevmoo Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Apr 7, 2026

Uh oh!

natebosch commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevmoo commented Apr 4, 2026 •

edited

Loading

github-actions Bot commented Apr 4, 2026 •

edited

Loading

github-actions Bot commented Apr 4, 2026 •

edited

Loading