X Tutup
The Wayback Machine - https://web.archive.org/web/20230102180204/https://github.com/nodejs/node/pull/46041
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

Jump to bottom

dgram: sync the old handle state to new handle #46041

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

dgram: sync the old handle state to new handle #46041

wants to merge 1 commit into from
+27 −1

Conversation

theanarkh
Copy link
Contributor

@theanarkh theanarkh commented Jan 1, 2023

Sync the old handle state to new handle Or the unref UDP socket will keep the loop alive.

  • 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
Contributor

nodejs-github-bot commented Jan 1, 2023

Review requested:

@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. labels Jan 1, 2023
mcollina
mcollina approved these changes Jan 1, 2023
View changes
Copy link
Member

@mcollina mcollina left a comment

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Jan 1, 2023

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

@targos
Copy link
Member

targos commented Jan 1, 2023

The added test fails on Windows: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/18447/RUN_SUBSET=3,nodes=win2012r2-COMPILED_BY-vs2019-x86/testReport/junit/(root)/test/parallel_test_dgram_unref_in_cluster/

ShogunPanda
ShogunPanda approved these changes Jan 1, 2023
View changes
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

LGTM!

@theanarkh
Copy link
Contributor Author

theanarkh commented Jan 1, 2023

The added test fails on Windows: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/18447/RUN_SUBSET=3,nodes=win2012r2-COMPILED_BY-vs2019-x86/testReport/junit/(root)/test/parallel_test_dgram_unref_in_cluster/

Yes, i have updated the test, and test it again.

@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 1, 2023
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Jan 1, 2023

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

@theanarkh
Copy link
Contributor Author

theanarkh commented Jan 1, 2023

using dgram in cluster is currently not supported on Windows.

cjihrig
cjihrig approved these changes Jan 1, 2023
View changes
@github-actions github-actions bot mentioned this pull request Jan 2, 2023
Open
22 tasks
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Jan 2, 2023

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

RaisinTen
RaisinTen approved these changes Jan 2, 2023
View changes
test/parallel/test-dgram-unref-in-cluster.js Outdated Show resolved Hide resolved
@theanarkh theanarkh added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2023
@nodejs-github-bot
Copy link
Contributor

nodejs-github-bot commented Jan 2, 2023

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

juanarbol
juanarbol approved these changes Jan 2, 2023
View changes
Copy link
Member

@juanarbol juanarbol left a comment

LGTM

@theanarkh theanarkh added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina

@ShogunPanda ShogunPanda

@addaleax addaleax

@cjihrig cjihrig

@juanarbol juanarbol

@RaisinTen RaisinTen

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@theanarkh @nodejs-github-bot @targos @mcollina @ShogunPanda @addaleax @cjihrig @juanarbol @RaisinTen
X Tutup