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

[v18.x] build: bring back dtrace/systemtap support on install #44643

Closed
wants to merge 16 commits into from

Conversation

RafaelGSS
Copy link
Member

See #44521 (comment).

It was accidentally merged into v18.9.0 (it might be a bad conflict resolution or even a cherry-pick mistake).

Shall we land a patch release or we can wait until v18.10 (2022-09-20)?

cc: @nodejs/releasers

bnoordhuis and others added 16 commits September 7, 2022 09:20
This flag was added back in 2013 to support postmortem debugging on
Linux but it has a pretty bad impact on performance (up to 10%) and
I don't think it's actually necessary to get meaningful stack traces.
Maybe with mdb but gdb and lldb seem to manage just fine.

Even if the flag is necessary for postmortem debugging, I don't think
it's appropriate to make performance for all users suffer for the
benefit of a fringe group.

PR-URL: nodejs#44452
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
`Environment::GetCurrent` may not available in the context of OOM.
Removes the cyclic `Environment::GetCurrent` and `env->isolate()`
calls to ensure both `isolate` and `env` is present if available.

However, this behavior is not guaranteed. As
`Environment::GetCurrent` didn't allocate new handles in the heap,
when a Context is entered it can still get the valid env pointer.
Removes the unstable assertion of the absence of env in the test.

PR-URL: nodejs#44398
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
When a new context with a different security token is entered, or
when no context is entered, `StackTrace::CurrentStackTrace` need to
be explicitly set with flag `kExposeFramesAcrossSecurityOrigins` to
avoid crashing.

PR-URL: nodejs#44398
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs#44486
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
This is not a function and should not use the term "return" to describe
its type or value.

PR-URL: nodejs#44481
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
We already do this in some places. This adds the digest name to
remaining uses of ERR_CRYPTO_INVALID_DIGEST except for one occurrence in
crypto_sig.cc that would require significant refactoring due to the
unusual error handling there.

PR-URL: nodejs#44468
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fixes nodejs#44471

PR-URL: nodejs#44475
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
In 14.2 in the specification, `error` should be signal’s abort reason.
The current behavior seems to assume that only an `AbortError` instance
is given as signal’s abort reason.

Refs: https://streams.spec.whatwg.org/#readable-stream-pipe-to

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
PR-URL: nodejs#44418
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#44450
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
PR-URL: nodejs#44511
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
PR-URL: nodejs#44510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Despite being named onend_arg, the pointer is passed both to the
onend_cb and to the onhello_cb. Rename it to cb_arg, which matches the
name of the class field cb_arg_.

PR-URL: nodejs#44500
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs#44512
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#44509
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Sep 14, 2022
@RafaelGSS
Copy link
Member Author

My fork base is dirty. I'll create a new fresh PR.

@RafaelGSS RafaelGSS closed this Sep 14, 2022
@RafaelGSS
Copy link
Member Author

See: #44642

@AdamMajer
Copy link
Contributor

Also affects 16.x now via e96bb14942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants