fix(cdk-assets-lib): pLimit.dispose() doesn't stop new jobs from starting - #1877
Open
Adityaj0 wants to merge 1 commit into
Open
fix(cdk-assets-lib): pLimit.dispose() doesn't stop new jobs from starting#1877Adityaj0 wants to merge 1 commit into
Adityaj0 wants to merge 1 commit into
Conversation
…ting dispatch() never checked the `stopped` flag, only resumeNext() (called when an already-active job finishes) did. So calling dispose() and then submitting a new task via the limiter, while a concurrency slot happened to be free, would start that task immediately instead of rejecting it - silently violating dispose()'s documented/tested contract that "new jobs aren't started after dispose is called". Guard dispatch() with the stopped flag, and reject immediately when a task is submitted after dispose() so callers don't hang waiting on a promise that will never settle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adityaj0
requested a deployment
to
integ-approval
August 22, 2026 06:38 — with
GitHub Actions
Waiting
aws-cdk-automation
enabled auto-merge
August 22, 2026 06:38
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1877 +/- ##
=======================================
Coverage 91.10% 91.10%
=======================================
Files 80 80
Lines 12205 12205
Branches 1742 1742
=======================================
Hits 11119 11119
Misses 1050 1050
Partials 36 36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1876
Reason for this change
pLimit()'sdispose()method is documented and tested to stop new jobs from starting, butdispatch()(called every time a new task is submitted) never checked thestoppedflag - onlyresumeNext()(which runs when an already-active job finishes) did. So a task submitted afterdispose()would start immediately whenever a concurrency slot happened to be free at submission time, silently violatingdispose()'s contract.Description of changes
dispatch()now returns immediately ifstoppedis true, so no new job starts afterdispose().ret()now rejects immediately with'Task has been cancelled'if a task is submitted afterdispose(), so callers don't hang waiting on a promise that would otherwise never settle (queued-but-never-dispatched).test/private/p-limit.test.tsthat fails before the fix (job runs despitedispose()) and passes after.Description of how you validated changes
Ran the full
@aws-cdk/cdk-assets-libtest suite locally - all 104 tests across 14 suites pass, including the new regression test and the existingp-limit.test.tstests.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license