Skip to content

net: fix connect crash when call destroy in lookup handler#51826

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 1 commit into from
Feb 27, 2024
Merged

net: fix connect crash when call destroy in lookup handler #51826

nodejs-github-bot merged 1 commit into from
Feb 27, 2024

Conversation

@theanarkh
Copy link
Contributor

@theanarkhtheanarkh commented Feb 21, 2024•
edited
Loading

fix: #50841

cc @ShogunPanda

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 21, 2024

Review requested:

  • @nodejs/net

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Feb 21, 2024
@theanarkhtheanarkhforce-pushed the fix_connect_crash_when_call_destroy_in_lookup_handler branch from 6a7c817 to 18718f4CompareFebruary 21, 2024 18:11
ShogunPanda
ShogunPanda approved these changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@ShogunPandaShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@theanarkhtheanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 22, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/57296/

@theanarkhtheanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 22, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/57297/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 22, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/57304/

lpinca
lpinca approved these changes Feb 22, 2024
View reviewed changes
mcollina
mcollina approved these changes Feb 23, 2024
View reviewed changes
Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@theanarkhtheanarkhforce-pushed the fix_connect_crash_when_call_destroy_in_lookup_handler branch from 18718f4 to 23b04caCompareFebruary 23, 2024 10:14
@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 23, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/57340/

@theanarkhtheanarkh added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 23, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2024
@theanarkhtheanarkh added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 24, 2024

Commit Queue failed
- Loading data for nodejs/node/pull/51826 ✔ Done loading data for nodejs/node/pull/51826 ----------------------------------- PR info ------------------------------------ Title net: fix connect crash when call destroy in lookup handler (#51826) Author theanarkh (@theanarkh) Branch theanarkh:fix_connect_crash_when_call_destroy_in_lookup_handler -> nodejs:main Labels net, author ready, needs-ci Commits 1 - net: fix connect crash when call destroy in lookup handler Committers 1 - theanarkh PR-URL: https://github.com/nodejs/node/pull/51826 Fixes: https://github.com/nodejs/node/issues/50841 Reviewed-By: Paolo Insogna Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/51826 Fixes: https://github.com/nodejs/node/issues/50841 Reviewed-By: Paolo Insogna Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - net: fix connect crash when call destroy in lookup handler ℹ This PR was created on Wed, 21 Feb 2024 17:12:43 GMT ✔ Approvals: 3 ✔ - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/51826#pullrequestreview-1895223170 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51826#pullrequestreview-1896819310 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/51826#pullrequestreview-1897763462 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-02-23T10:16:28Z: https://ci.nodejs.org/job/node-test-pull-request/57340/ - Querying data for job/node-test-pull-request/57340/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8028025584

@nodejs-github-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 24, 2024
@theanarkhtheanarkh added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 27, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 27, 2024
@nodejs-github-botnodejs-github-bot merged commit 292016c into nodejs:mainFeb 27, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 27, 2024

Landed in 292016c

marco-ippolito pushed a commit that referenced this pull request Feb 29, 2024
@theanarkh@marco-ippolito
net: fix connect crash when call destroy in lookup handler
55f27eb
PR-URL: #51826Fixes: #50841 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Feb 29, 2024
@theanarkh@marco-ippolito
net: fix connect crash when call destroy in lookup handler
c6cc3ed
PR-URL: #51826Fixes: #50841 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
@marco-ippolitomarco-ippolito mentioned this pull request Mar 1, 2024
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@theanarkh@richardlau
net: fix connect crash when call destroy in lookup handler
87919a2
PR-URL: #51826Fixes: #50841 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@theanarkh@richardlau
net: fix connect crash when call destroy in lookup handler
870ef5a
PR-URL: #51826Fixes: #50841 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Merged
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
@theanarkh@rdw-msft
net: fix connect crash when call destroy in lookup handler
5601dfc
PR-URL: nodejs#51826Fixes: nodejs#50841 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes

@ShogunPandaShogunPandaShogunPanda approved these changes

@lpincalpincalpinca approved these changes

@gengjiawengengjiawengengjiawen approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-failedAn error occurred while landing this pull request using GitHub Actions.needs-ciPRs that need a full CI run.netIssues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Node js internal assertion error in socket

6 participants

@theanarkh@nodejs-github-bot@mcollina@ShogunPanda@lpinca@gengjiawen