Skip to content
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

fs: fix nonNativeWatcher leak of StatWatchers #45501

Merged
merged 1 commit into from
Nov 27, 2022

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Nov 17, 2022

a simple reproduction was added as a test

@MoLow MoLow requested a review from anonrig November 17, 2022 22:27
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Nov 17, 2022
@MoLow
Copy link
Member Author

MoLow commented Nov 17, 2022

CC @nodejs/fs

test/parallel/test-fs-watch-recursive.js Outdated Show resolved Hide resolved
test/parallel/test-fs-watch-recursive.js Show resolved Hide resolved
test/parallel/test-fs-watch-recursive.js Show resolved Hide resolved
@MoLow MoLow requested a review from anonrig November 25, 2022 07:28
@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Nov 25, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 25, 2022
@nodejs-github-bot
Copy link
Collaborator

@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 27, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45501
✔  Done loading data for nodejs/node/pull/45501
----------------------------------- PR info ------------------------------------
Title      fs: fix `nonNativeWatcher` leak of `StatWatchers` (#45501)
Author     Moshe Atlow  (@MoLow)
Branch     MoLow:fix-recursive-watch-leak -> nodejs:main
Labels     fs, author ready, needs-ci, commit-queue-squash
Commits    6
 - fs: fix `nonNativeWatcher` leak of `StatWatchers`
 - Lint
 - fix
 - CR
 - CR
 - fix
Committers 1
 - Moshe Atlow 
PR-URL: https://github.com/nodejs/node/pull/45501
Reviewed-By: Yagiz Nizipli 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45501
Reviewed-By: Yagiz Nizipli 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fix
   ℹ  This PR was created on Thu, 17 Nov 2022 22:27:07 GMT
   ✔  Approvals: 1
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/45501#pullrequestreview-1194715945
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-11-26T22:27:48Z: https://ci.nodejs.org/job/node-test-pull-request/48201/
- Querying data for job/node-test-pull-request/48201/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3557422926

PR-URL: nodejs#45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@MoLow MoLow force-pushed the fix-recursive-watch-leak branch from 665c6f9 to b9fbd1c Compare November 27, 2022 08:39
@MoLow MoLow merged commit b9fbd1c into nodejs:main Nov 27, 2022
@MoLow
Copy link
Member Author

MoLow commented Nov 27, 2022

Landed in b9fbd1c

@MoLow MoLow deleted the fix-recursive-watch-leak branch November 27, 2022 08:40
ErickWendel pushed a commit to ErickWendel/node that referenced this pull request Nov 30, 2022
PR-URL: nodejs#45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
targos pushed a commit that referenced this pull request Dec 12, 2022
PR-URL: #45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@danielleadams
Copy link
Contributor

Hey @MoLow, the test couldn't be pulled into v18.x-staging. Do you mind creating a backport PR? Thanks!

@danielleadams danielleadams added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Dec 30, 2022
MoLow added a commit to MoLow/node that referenced this pull request Dec 31, 2022
PR-URL: nodejs#45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@MoLow MoLow added backport-open-v18.x Indicate that the PR has an open backport. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels Dec 31, 2022
MoLow added a commit to MoLow/node that referenced this pull request Jan 5, 2023
PR-URL: nodejs#45501
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-open-v18.x Indicate that the PR has an open backport. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants