Skip to content

dgram and tls close() cleanup wrt. callbacks - #217

Closed
sam-github wants to merge 3 commits into
nodejs:v0.12from
sam-github:dgram-close-async-emit
Closed

dgram and tls close() cleanup wrt. callbacks#217
sam-github wants to merge 3 commits into
nodejs:v0.12from
sam-github:dgram-close-async-emit

Conversation

@sam-github

Copy link
Copy Markdown
Contributor
  • dgram: make 'close' event async
  • dgram: close() should accept a callback
  • doc: add tls server.close() callback docs

@sam-github

Copy link
Copy Markdown
ContributorAuthor

@piscisaureus Above changes may make promisification (in-core or thirdparty) smoother by increasing the number of core APIs that use the callback last convention, rather than triggering a one-time event on completion of the action. If it wasn't so late, I'd dig up the rant in the promisification thread about one-time events being an anti-pattern... I kindof agree, though I also use EE as a kind of Observer pattern, so I kindof don't. :-)

@sam-github
sam-githubforce-pushed the dgram-close-async-emit branch 2 times, most recently from 89cba90 to 9b09606CompareDecember 30, 2014 18:00
Comment threadtest/parallel/test-dgram-close.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor nit: don't check if code === 0, we don't do that anywhere else. Sorry, missed that one last time.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It avoids duplicate stack traces on failure, but sure, deleted.

Emit the close event asynchronously, after the close, as it is with the
net/http close events.
Like net, http, and https server.close, and socket.end(), etc.
Also, tests to confirm its existence.
@sam-github
sam-githubforce-pushed the dgram-close-async-emit branch from 9b09606 to 61ac711CompareDecember 30, 2014 20:10
bnoordhuis pushed a commit that referenced this pull request Dec 30, 2014
Emit the close event asynchronously, after the close, as it is with the
net/http close events.
PR-URL: #217
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
bnoordhuis pushed a commit that referenced this pull request Dec 30, 2014
Like net, http, and https server.close, and socket.end(), etc.
PR-URL: #217
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
bnoordhuis pushed a commit that referenced this pull request Dec 30, 2014
Also, tests to confirm its existence.
PR-URL: #217
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis

Copy link
Copy Markdown
Member

Cheers Sam, landed in commits 7349d7f to b42c085.

@sam-github

Copy link
Copy Markdown
ContributorAuthor

Thanks, Ben.

@sam-github
sam-github deleted the dgram-close-async-emit branch December 30, 2014 21:24
@rvaggrvagg mentioned this pull request Jan 3, 2015
Trott pushed a commit to npm/node that referenced this pull request Aug 20, 2019
Full release notes:
A few meaty bugfixes, and introducing `peerDependenciesMeta`.
FEATURES
* [`a12341088`](npm/cli@a123410)
[nodejs#224](npm/cli#224) Implements
peerDependenciesMeta ([@arcanis](https://github.com/arcanis))
* [`2f3b79bba`](npm/cli@2f3b79b)
[nodejs#234](npm/cli#234) add new forbidden 403
error code ([@claudiahdz](https://github.com/claudiahdz))
BUGFIXES
* [`24acc9fc8`](npm/cli@24acc9f)
and
[`45772af0d`](npm/cli@45772af)
[nodejs#217](npm/cli#217)
[npm.community#8863](https://npm.community/t/installing-the-same-module-under-multiple-relative-paths-fails-on-linux/8863)
[npm.community#9327](https://npm.community/t/reinstall-breaks-after-npm-update-to-6-10-2/9327,)
do not descend into directory deps' child modules, fix shrinkwrap
files that inappropriately list child nodes of symlink packages
([@isaacs](https://github.com/isaacs) and
[@salomvary](https://github.com/salomvary))
* [`50cfe113d`](npm/cli@50cfe11)
[nodejs#229](npm/cli#229) fixed typo in semver doc
([@gall0ws](https://github.com/gall0ws))
* [`e8fb2a1bd`](npm/cli@e8fb2a1)
[nodejs#231](npm/cli#231) Fix spelling mistakes in
CHANGELOG-3.md ([@XhmikosR](https://github.com/XhmikosR))
* [`769d2e057`](npm/cli@769d2e0)
[npm/uid-number#7](npm/uid-number#7) Better
error on invalid `--user`/`--group` configs. This addresses the issue
when people fail to install binary packages on Docker and other
environments where there is no 'nobody' user.
([@isaacs](https://github.com/isaacs))
* [`8b43c9624`](npm/cli@8b43c96)
[nodejs#28987](nodejs#28987)
[npm.community#6032](https://npm.community/t/npm-ci-doesnt-respect-npmrc-variables/6032)
[npm.community#6658](https://npm.community/t/npm-ci-doesnt-fill-anymore-the-process-env-npm-config-cache-variable-on-post-install-scripts/6658)
[npm.community#6069](https://npm.community/t/npm-ci-does-not-compile-native-dependencies-according-to-npmrc-configuration/6069)
[npm.community#9323](https://npm.community/t/npm-6-9-x-not-passing-environment-to-node-gyp-regression-from-6-4-x/9323/2)
Fix the regression where random config values in a .npmrc file are not
passed to lifecycle scripts, breaking build processes which rely on
them. ([@isaacs](https://github.com/isaacs))
* [`8b85eaa47`](npm/cli@8b85eaa)
save files with inferred ownership rather than relying on `SUDO_UID`
and `SUDO_GID`. ([@isaacs](https://github.com/isaacs))
* [`b7f6e5f02`](npm/cli@b7f6e5f)
Infer ownership of shrinkwrap files
([@isaacs](https://github.com/isaacs))
* [`54b095d77`](npm/cli@54b095d)
[nodejs#235](npm/cli#235) Add spec to dist-tag
remove function ([@theberbie](https://github.com/theberbie))
DEPENDENCIES
* [`dc8f9e52f`](npm/cli@dc8f9e5)
`pacote@9.5.7`: Infer the ownership of all unpacked files in
`node_modules`, so that we never have user-owned files in root-owned
folders, or root-owned files in user-owned folders.
([@isaacs](https://github.com/isaacs))
* [`bb33940c3`](npm/cli@bb33940)
`cmd-shim@3.0.0`:
* [`9c93ac3`](npm/cmd-shim@9c93ac3)
[#2](npm/cmd-shim#2)
[npm#3380](npm/npm#3380) Handle
environment variables properly
([@basbossink](https://github.com/basbossink))
* [`2d277f8`](npm/cmd-shim@2d277f8)
[nodejs#25](npm/cmd-shim#25)
[nodejs#36](npm/cmd-shim#36)
[nodejs#35](npm/cmd-shim#35) Fix 'no shebang' case
by always providing `$basedir` in shell script
([@igorklopov](https://github.com/igorklopov))
* [`adaf20b`](npm/cmd-shim@adaf20b)
[nodejs#26](npm/cmd-shim#26) Fix `$*` causing an
error when arguments contain parentheses
([@satazor](https://github.com/satazor))
* [`49f0c13`](npm/cmd-shim@49f0c13)
[nodejs#30](npm/cmd-shim#30) Fix paths for
MSYS/MINGW bash ([@dscho](https://github.com/dscho))
* [`51a8af3`](npm/cmd-shim@51a8af3)
[nodejs#34](npm/cmd-shim#34) Add proper support
for PowerShell ([@ExE-Boss](https://github.com/ExE-Boss))
* [`4c37e04`](npm/cmd-shim@4c37e04)
[#10](npm/cmd-shim#10) Work around quoted
batch file names ([@isaacs](https://github.com/isaacs))
* [`a4e279544`](npm/cli@a4e2795)
`npm-lifecycle@3.1.3` ([@isaacs](https://github.com/isaacs)):
* fail properly if `uid-number` raises an error
* [`7086a1809`](npm/cli@7086a18)
`libcipm@4.0.3` ([@isaacs](https://github.com/isaacs))
* [`8845141f9`](npm/cli@8845141)
`read-package-json@2.1.0` ([@isaacs](https://github.com/isaacs))
* [`51c028215`](npm/cli@51c0282)
`bin-links@1.1.3` ([@isaacs](https://github.com/isaacs))
* [`534a5548c`](npm/cli@534a554)
`read-cmd-shim@1.0.3` ([@isaacs](https://github.com/isaacs))
* [`3038f2fd5`](npm/cli@3038f2f)
`gentle-fs@2.2.1` ([@isaacs](https://github.com/isaacs))
* [`a609a1648`](npm/cli@a609a16)
`graceful-fs@4.2.2` ([@isaacs](https://github.com/isaacs))
* [`f0346f754`](npm/cli@f0346f7)
`cacache@12.0.3` ([@isaacs](https://github.com/isaacs))
* [`ca9c615c8`](npm/cli@ca9c615)
`npm-pick-manifest@3.0.0` ([@isaacs](https://github.com/isaacs))
* [`b417affbf`](npm/cli@b417aff)
`pacote@9.5.8` ([@isaacs](https://github.com/isaacs))
TESTS
* [`b6df0913c`](npm/cli@b6df091)
[nodejs#228](npm/cli#228) Proper handing of
/usr/bin/node lifecycle-path test
([@olivr70](https://github.com/olivr70))
* [`aaf98e88c`](npm/cli@aaf98e8)
`npm-registry-mock@1.3.0` ([@isaacs](https://github.com/isaacs))
isaacs added a commit to npm/node that referenced this pull request Aug 21, 2019
Full changelog:
6.11.1 (2019-08-20):
Fix a regression for windows command shim syntax.
* [`37db29647`](npm/cli@37db296)
`cmd-shim@3.0.2` ([@isaacs](https://github.com/isaacs))
v6.11.0 (2019-08-20):
A few meaty bugfixes, and introducing `peerDependenciesMeta`.
FEATURES
* [`a12341088`](npm/cli@a123410)
[nodejs#224](npm/cli#224) Implements
peerDependenciesMeta ([@arcanis](https://github.com/arcanis))
* [`2f3b79bba`](npm/cli@2f3b79b)
[nodejs#234](npm/cli#234) add new forbidden 403 error
code ([@claudiahdz](https://github.com/claudiahdz))
BUGFIXES
* [`24acc9fc8`](npm/cli@24acc9f)
and
[`45772af0d`](npm/cli@45772af)
[nodejs#217](npm/cli#217)
[npm.community#8863](https://npm.community/t/installing-the-same-module-under-multiple-relative-paths-fails-on-linux/8863)
[npm.community#9327](https://npm.community/t/reinstall-breaks-after-npm-update-to-6-10-2/9327,)
do not descend into directory deps' child modules, fix shrinkwrap files
that inappropriately list child nodes of symlink packages
([@isaacs](https://github.com/isaacs) and
[@salomvary](https://github.com/salomvary))
* [`50cfe113d`](npm/cli@50cfe11)
[nodejs#229](npm/cli#229) fixed typo in semver doc
([@gall0ws](https://github.com/gall0ws))
* [`e8fb2a1bd`](npm/cli@e8fb2a1)
[nodejs#231](npm/cli#231) Fix spelling mistakes in
CHANGELOG-3.md ([@XhmikosR](https://github.com/XhmikosR))
* [`769d2e057`](npm/cli@769d2e0)
[npm/uid-number#7](npm/uid-number#7) Better
error on invalid `--user`/`--group` configs. This addresses the issue
when people fail to install binary packages on Docker and other
environments where there is no 'nobody' user.
([@isaacs](https://github.com/isaacs))
* [`8b43c9624`](npm/cli@8b43c96)
[nodejs#28987](nodejs#28987)
[npm.community#6032](https://npm.community/t/npm-ci-doesnt-respect-npmrc-variables/6032)
[npm.community#6658](https://npm.community/t/npm-ci-doesnt-fill-anymore-the-process-env-npm-config-cache-variable-on-post-install-scripts/6658)
[npm.community#6069](https://npm.community/t/npm-ci-does-not-compile-native-dependencies-according-to-npmrc-configuration/6069)
[npm.community#9323](https://npm.community/t/npm-6-9-x-not-passing-environment-to-node-gyp-regression-from-6-4-x/9323/2)
Fix the regression where random config values in a .npmrc file are not
passed to lifecycle scripts, breaking build processes which rely on them.
([@isaacs](https://github.com/isaacs))
* [`8b85eaa47`](npm/cli@8b85eaa)
save files with inferred ownership rather than relying on `SUDO_UID` and
`SUDO_GID`. ([@isaacs](https://github.com/isaacs))
* [`b7f6e5f02`](npm/cli@b7f6e5f)
Infer ownership of shrinkwrap files
([@isaacs](https://github.com/isaacs))
* [`54b095d77`](npm/cli@54b095d)
[nodejs#235](npm/cli#235) Add spec to dist-tag remove
function ([@theberbie](https://github.com/theberbie))
DEPENDENCIES
* [`dc8f9e52f`](npm/cli@dc8f9e5)
`pacote@9.5.7`: Infer the ownership of all unpacked files in
`node_modules`, so that we never have user-owned files in root-owned
folders, or root-owned files in user-owned folders.
([@isaacs](https://github.com/isaacs))
* [`bb33940c3`](npm/cli@bb33940)
`cmd-shim@3.0.0`:
* [`9c93ac3`](npm/cmd-shim@9c93ac3)
[#2](npm/cmd-shim#2)
[npm#3380](npm/npm#3380) Handle environment
variables properly ([@basbossink](https://github.com/basbossink))
* [`2d277f8`](npm/cmd-shim@2d277f8)
[nodejs#25](npm/cmd-shim#25)
[nodejs#36](npm/cmd-shim#36)
[nodejs#35](npm/cmd-shim#35) Fix 'no shebang' case by
always providing `$basedir` in shell script
([@igorklopov](https://github.com/igorklopov))
* [`adaf20b`](npm/cmd-shim@adaf20b)
[nodejs#26](npm/cmd-shim#26) Fix `$*` causing an
error when arguments contain parentheses
([@satazor](https://github.com/satazor))
* [`49f0c13`](npm/cmd-shim@49f0c13)
[nodejs#30](npm/cmd-shim#30) Fix paths for MSYS/MINGW
bash ([@dscho](https://github.com/dscho))
* [`51a8af3`](npm/cmd-shim@51a8af3)
[nodejs#34](npm/cmd-shim#34) Add proper support for
PowerShell ([@ExE-Boss](https://github.com/ExE-Boss))
* [`4c37e04`](npm/cmd-shim@4c37e04)
[#10](npm/cmd-shim#10) Work around quoted
batch file names ([@isaacs](https://github.com/isaacs))
* [`a4e279544`](npm/cli@a4e2795)
`npm-lifecycle@3.1.3` ([@isaacs](https://github.com/isaacs)):
* fail properly if `uid-number` raises an error
* [`7086a1809`](npm/cli@7086a18)
`libcipm@4.0.3` ([@isaacs](https://github.com/isaacs))
* [`8845141f9`](npm/cli@8845141)
`read-package-json@2.1.0` ([@isaacs](https://github.com/isaacs))
* [`51c028215`](npm/cli@51c0282)
`bin-links@1.1.3` ([@isaacs](https://github.com/isaacs))
* [`534a5548c`](npm/cli@534a554)
`read-cmd-shim@1.0.3` ([@isaacs](https://github.com/isaacs))
* [`3038f2fd5`](npm/cli@3038f2f)
`gentle-fs@2.2.1` ([@isaacs](https://github.com/isaacs))
* [`a609a1648`](npm/cli@a609a16)
`graceful-fs@4.2.2` ([@isaacs](https://github.com/isaacs))
* [`f0346f754`](npm/cli@f0346f7)
`cacache@12.0.3` ([@isaacs](https://github.com/isaacs))
* [`ca9c615c8`](npm/cli@ca9c615)
`npm-pick-manifest@3.0.0` ([@isaacs](https://github.com/isaacs))
* [`b417affbf`](npm/cli@b417aff)
`pacote@9.5.8` ([@isaacs](https://github.com/isaacs))
TESTS
* [`b6df0913c`](npm/cli@b6df091)
[nodejs#228](npm/cli#228) Proper handing of
/usr/bin/node lifecycle-path test
([@olivr70](https://github.com/olivr70))
* [`aaf98e88c`](npm/cli@aaf98e8)
`npm-registry-mock@1.3.0` ([@isaacs](https://github.com/isaacs))
syg pushed a commit to syg/node that referenced this pull request May 5, 2025
bakkot pushed a commit to bakkot/node that referenced this pull request May 11, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sam-github@bnoordhuis