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

wasi: throw on failed uvwasi_init() #31076

Merged
merged 3 commits into from Dec 26, 2019
Merged

wasi: throw on failed uvwasi_init() #31076

merged 3 commits into from Dec 26, 2019

Conversation

@cjihrig
Copy link
Contributor

cjihrig commented Dec 24, 2019

Prior to this commit, if uvwasi_init() failed in any way, Node would abort due to a failed CHECK_EQ(). This commit changes that behavior to a thrown exception.

Fixes: #30878

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@gengjiawen gengjiawen added the wasi label Dec 24, 2019
deps/uvwasi/src/fd_table.c Outdated Show resolved Hide resolved
src/node_wasi.cc Outdated Show resolved Hide resolved
src/node_wasi.cc Outdated Show resolved Hide resolved
@cjihrig cjihrig force-pushed the cjihrig:wasi-err branch from e146150 to 03eaa6c Dec 24, 2019
@cjihrig

This comment has been minimized.

Copy link
Contributor Author

cjihrig commented Dec 24, 2019

@addaleax I think I've addressed all of your comments.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link

nodejs-github-bot commented Dec 25, 2019

@Trott
Trott approved these changes Dec 25, 2019
cjihrig added a commit to cjihrig/node-1 that referenced this pull request Dec 25, 2019
Original commit message:

    This commit ensures that multiple calls to uvwasi_destroy()
    are possible. Prior to this commit, subsequent calls to
    destroy() would abort because the file table's rwlock was
    incorrectly being destroyed multiple times.

PR-URL: nodejs#31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
cjihrig added a commit to cjihrig/node-1 that referenced this pull request Dec 25, 2019
Original commit message:

    This commit changes the memory management in
    uvwasi_fd_table_init() such that ENOMEM errors can be
    handled better in uvwasi_fd_table_free().

PR-URL: nodejs#31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
cjihrig added a commit to cjihrig/node-1 that referenced this pull request Dec 25, 2019
Prior to this commit, if uvwasi_init() failed in any way, Node
would abort due to a failed CHECK_EQ(). This commit changes
that behavior to a thrown exception.

PR-URL: nodejs#31076
Fixes: nodejs#30878
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@cjihrig cjihrig force-pushed the cjihrig:wasi-err branch from d5cde42 to 57e0d8b Dec 25, 2019
@nodejs-github-bot

This comment has been minimized.

cjihrig added 3 commits Dec 22, 2019
Prior to this commit, if uvwasi_init() failed in any way, Node
would abort due to a failed CHECK_EQ(). This commit changes
that behavior to a thrown exception.

PR-URL: #31076
Fixes: #30878
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Original commit message:

    This commit ensures that multiple calls to uvwasi_destroy()
    are possible. Prior to this commit, subsequent calls to
    destroy() would abort because the file table's rwlock was
    incorrectly being destroyed multiple times.

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Original commit message:

    This commit changes the memory management in
    uvwasi_fd_table_init() such that ENOMEM errors can be
    handled better in uvwasi_fd_table_free().

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@cjihrig cjihrig force-pushed the cjihrig:wasi-err branch from 57e0d8b to 208453e Dec 26, 2019
@cjihrig cjihrig merged commit 208453e into nodejs:master Dec 26, 2019
0 of 2 checks passed
0 of 2 checks passed
Travis CI - Pull Request Build Errored
Details
Travis CI - Branch Build Created
Details
@cjihrig

This comment has been minimized.

Copy link
Contributor Author

cjihrig commented Dec 26, 2019

Landed in 3d50001...208453e. Thanks for the reviews.

@cjihrig cjihrig deleted the cjihrig:wasi-err branch Dec 26, 2019
BridgeAR added a commit that referenced this pull request Jan 3, 2020
Original commit message:

    This commit ensures that multiple calls to uvwasi_destroy()
    are possible. Prior to this commit, subsequent calls to
    destroy() would abort because the file table's rwlock was
    incorrectly being destroyed multiple times.

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit that referenced this pull request Jan 3, 2020
Original commit message:

    This commit changes the memory management in
    uvwasi_fd_table_init() such that ENOMEM errors can be
    handled better in uvwasi_fd_table_free().

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit that referenced this pull request Jan 3, 2020
Prior to this commit, if uvwasi_init() failed in any way, Node
would abort due to a failed CHECK_EQ(). This commit changes
that behavior to a thrown exception.

PR-URL: #31076
Fixes: #30878
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
targos added a commit that referenced this pull request Jan 14, 2020
Original commit message:

    This commit ensures that multiple calls to uvwasi_destroy()
    are possible. Prior to this commit, subsequent calls to
    destroy() would abort because the file table's rwlock was
    incorrectly being destroyed multiple times.

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
Original commit message:

    This commit changes the memory management in
    uvwasi_fd_table_init() such that ENOMEM errors can be
    handled better in uvwasi_fd_table_free().

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
Prior to this commit, if uvwasi_init() failed in any way, Node
would abort due to a failed CHECK_EQ(). This commit changes
that behavior to a thrown exception.

PR-URL: #31076
Fixes: #30878
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
Original commit message:

    This commit ensures that multiple calls to uvwasi_destroy()
    are possible. Prior to this commit, subsequent calls to
    destroy() would abort because the file table's rwlock was
    incorrectly being destroyed multiple times.

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
Original commit message:

    This commit changes the memory management in
    uvwasi_fd_table_init() such that ENOMEM errors can be
    handled better in uvwasi_fd_table_free().

PR-URL: #31076
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
Prior to this commit, if uvwasi_init() failed in any way, Node
would abort due to a failed CHECK_EQ(). This commit changes
that behavior to a thrown exception.

PR-URL: #31076
Fixes: #30878
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.