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

src: fix inspecting MessagePort from init async hook #31600

Closed
wants to merge 2 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Feb 1, 2020

During the init() async hook, the C++ object is not finished
creating yet (i.e. it is an AsyncWrap, but not yet a HandleWrap
or MessagePort). Accessing the handle_ field is not valid
in that case.

However, the custom inspect function for MessagePorts calls
HasRef() on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

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

@addaleax addaleax added async_hooks Issues and PRs related to the async hooks subsystem. worker Issues and PRs related to Worker support. lts-watch-v12.x labels Feb 1, 2020
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 1, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2020
@nodejs-github-bot
Copy link
Collaborator

addaleax added a commit that referenced this pull request Feb 5, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

PR-URL: #31600
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@addaleax
Copy link
Member Author

addaleax commented Feb 5, 2020

Landed in c1da4e4

@addaleax addaleax closed this Feb 5, 2020
@addaleax addaleax deleted the messageport-init-inspect branch February 5, 2020 14:52
codebytere pushed a commit that referenced this pull request Feb 17, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

PR-URL: #31600
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 15, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

PR-URL: #31600
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

PR-URL: #31600
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Mar 17, 2020
codebytere pushed a commit that referenced this pull request Mar 30, 2020
During the `init()` async hook, the C++ object is not finished
creating yet (i.e. it is an `AsyncWrap`, but not yet a `HandleWrap`
or `MessagePort`). Accessing the `handle_` field is not valid
in that case.

However, the custom inspect function for `MessagePort`s calls
`HasRef()` on the object, which would crash when the object
is not fully constructed. Fix that by guarding the access of
the libuv handle on that condition.

PR-URL: #31600
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants