GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — events: extract addAbortListener for safe internal use by atlowChemi · Pull Request #52081 · nodejs/node

Conversation

@atlowChemi

@atlowChemi atlowChemi commented Mar 14, 2024

Copy link
Copy Markdown
Member

Refs: #48596

Currently we declare and use addAbortSignal on the public EventEmitter module, which means it can be overriden and changed by user-land, which can cause some unexpected behavior.
This PR extracts addAbortSignal to an internal module to ensure internal usage has the expected function.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and rluvaton reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@nodejs-github-bot

nodejs-github-bot commented Mar 14, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net
  • @nodejs/streams
  • @nodejs/test_runner
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Mar 14, 2024
@atlowChemi
atlowChemi force-pushed the safe-addAbortListener branch from f86c737 to eb934cc Compare March 14, 2024 09:18
aduh95
aduh95 reviewed Mar 14, 2024
View reviewed changes
Comment thread lib/internal/events/abort_listener.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@atlowChemi
atlowChemi force-pushed the safe-addAbortListener branch from ab1db82 to 1b9a76f Compare March 14, 2024 10:56
MoLow
MoLow approved these changes Mar 14, 2024
View reviewed changes
benjamingr
benjamingr approved these changes Mar 14, 2024
View reviewed changes
aduh95
aduh95 approved these changes Mar 14, 2024
View reviewed changes
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Mar 14, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 14, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 14, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca
lpinca approved these changes Mar 14, 2024
View reviewed changes
@atlowChemi atlowChemi added commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. commit-queue PRs queued for automated landing through the Commit Queue. labels Mar 14, 2024
atlowChemi added 9 commits March 15, 2024 17:15
@atlowChemi
events: extract addAbortListener for safe internal use
29ee68a
Refs: nodejs#48596
@atlowChemi
child_process: use internal addAbortListener
814ff46
@atlowChemi
dgram: use internal addAbortListener
0e2ac27
@atlowChemi
net: use internal addAbortListener
0169b22
@atlowChemi
readline: use internal addAbortListener
8260223
@atlowChemi
http2: use internal addAbortListener
4a76d0d
@atlowChemi
stream: use internal addAbortListener
54b7633
@atlowChemi
test_runner: use internal addAbortListener
be86203
@atlowChemi
watch: use internal addAbortListener
4b7ac75
@atlowChemi
atlowChemi force-pushed the safe-addAbortListener branch from 1b9a76f to 4b7ac75 Compare March 15, 2024 15:15
@atlowChemi atlowChemi added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Mar 15, 2024
@atlowChemi
fixup! events: extract addAbortListener for safe internal use
3117855
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 15, 2024
@RafaelGSS RafaelGSS mentioned this pull request Apr 15, 2024
2024-04-24, Version 22.0.0 (Current) #52505
Merged
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
events: extract addAbortListener for safe internal use
e57dc01
Refs: #48596
PR-URL: #52081
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
child_process: use internal addAbortListener
ffe2e6a
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
dgram: use internal addAbortListener
d592c58
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
net: use internal addAbortListener
f668734
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
readline: use internal addAbortListener
22721fb
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
http2: use internal addAbortListener
259365e
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
stream: use internal addAbortListener
7d2f5cc
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
test_runner: use internal addAbortListener
8f7e629
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@atlowChemi @marco-ippolito
watch: use internal addAbortListener
f9097a8
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request May 2, 2024
v20.13.0 proposal #52793
Merged
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
events: extract addAbortListener for safe internal use
5e69673
Refs: #48596
PR-URL: #52081
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
child_process: use internal addAbortListener
9af15cf
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
dgram: use internal addAbortListener
27d3644
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
net: use internal addAbortListener
37abad8
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
readline: use internal addAbortListener
fb601c3
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
http2: use internal addAbortListener
9f88736
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
stream: use internal addAbortListener
8442457
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
test_runner: use internal addAbortListener
68187c4
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@atlowChemi @marco-ippolito
watch: use internal addAbortListener
8ba6f9b
PR-URL: #52081
Refs: #48596
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request May 10, 2024
@codebytere
events: extract addAbortListener for safe internal use
e3ece91
nodejs/node#52081
@mcollina mcollina mentioned this pull request May 13, 2024
HTTP2 server memory explodes and terminated #52952
Closed
jkleinsc pushed a commit to electron/electron that referenced this pull request May 13, 2024
@electron-roller @codebytere @patchup
chore: bump node to v20.13.1 (main) (#42088)
653d0f0
* chore: bump node in DEPS to v20.13.0

* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL

nodejs/node#52217

* test: skip test for dynamically linked OpenSSL

nodejs/node#52542

* lib, url: add a `windows` option to path parsing

nodejs/node#52509

* src: use dedicated routine to compile function for builtin CJS loader

nodejs/node#52016

* test: mark test as flaky

nodejs/node#52671

* build,tools: add test-ubsan ci

nodejs/node#46297

* src: preload function for Environment

nodejs/node#51539

* chore: fixup patch indices

* deps: update c-ares to 1.28.1

nodejs/node#52285

* chore: handle updated filenames

- nodejs/node#51999
- nodejs/node#51927

* chore: bump node in DEPS to v20.13.1

* events: extract addAbortListener for safe internal use

nodejs/node#52081

* module: print location of unsettled top-level await in entry points

nodejs/node#51999

* fs: add stacktrace to fs/promises

nodejs/node#49849

* chore: update patches

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
@codebytere
events: extract addAbortListener for safe internal use
aa71a99
nodejs/node#52081
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
@codebytere
events: extract addAbortListener for safe internal use
424c434
nodejs/node#52081
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
@electron-roller @codebytere
chore: bump node to v20.14.0 (31-x-y) (#42295)
9a87938
* chore: bump node in DEPS to v20.13.1

* chore: bump node in DEPS to v20.14.0

* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL

nodejs/node#52217

* test: skip test for dynamically linked OpenSSL

nodejs/node#52542

* lib, url: add a `windows` option to path parsing

nodejs/node#52509

* src: use dedicated routine to compile function for builtin CJS loader

nodejs/node#52016

* test: mark test as flaky

nodejs/node#52671

* build,tools: add test-ubsan ci

nodejs/node#46297

* src: preload function for Environment

nodejs/node#51539

* chore: fixup patch indices

* deps: update c-ares to 1.28.1

nodejs/node#52285

* chore: handle updated filenames

* events: extract addAbortListener for safe internal use

nodejs/node#52081

* module: print location of unsettled top-level await in entry points

nodejs/node#51999

* fs: add stacktrace to fs/promises

nodejs/node#49849

* chore: fixup patch indices

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2024
@electron-roller @zcbenz
@codebytere @patchup
chore: bump node to v20.14.0 (30-x-y) (#42294)
3be3381
* chore: bump node in DEPS to v20.13.1

* chore: bump node in DEPS to v20.14.0

* chore: update build_add_gn_build_files.patch

* chore: update patches

* chore: update patches

* build: encode non-ASCII Latin1 characters as one byte in JS2C

nodejs/node#51605

* crypto: use EVP_MD_fetch and cache EVP_MD for hashes

nodejs/node#51034

* chore: update filenames.json

* chore: update patches

* src: support configurable snapshot

nodejs/node#50453

* test: remove test-domain-error-types flaky designation

nodejs/node#51717

* src: avoid draining platform tasks at FreeEnvironment

nodejs/node#51290

* chore: fix accidentally deleted v8 dep

* lib: define FormData and fetch etc. in the built-in snapshot

nodejs/node#51598

* chore: remove stray log

* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL

nodejs/node#52217

* test: skip test for dynamically linked OpenSSL

nodejs/node#52542

* lib, url: add a `windows` option to path parsing

nodejs/node#52509

* src: use dedicated routine to compile function for builtin CJS loader

nodejs/node#52016

* test: mark test as flaky

nodejs/node#52671

* build,tools: add test-ubsan ci

nodejs/node#46297

* src: preload function for Environment

nodejs/node#51539

* deps: update c-ares to 1.28.1

nodejs/node#52285

* chore: fixup

* events: extract addAbortListener for safe internal use

nodejs/node#52081

* module: print location of unsettled top-level await in entry points

nodejs/node#51999

* fs: add stacktrace to fs/promises

nodejs/node#49849

* chore: fixup indices

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Cheng <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingr benjamingr benjamingr approved these changes
@lpinca lpinca lpinca approved these changes
@MoLow MoLow MoLow approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. 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.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@atlowChemi @nodejs-github-bot @benjamingr @lpinca @MoLow @aduh95
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.