Skip to content

Conversation

@cjihrig
Copy link
Contributor

@cjihrigcjihrig 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

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 24, 2019
@gengjiawengengjiawen added the wasi Issues and PRs related to the WebAssembly System Interface. label Dec 24, 2019
@cjihrig
Copy link
ContributorAuthor

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

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 24, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 25, 2019

@nodejs-github-bot
Copy link
Collaborator

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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
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#31076Fixes: nodejs#30878 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@cjihrigcjihrig merged commit 208453e into nodejs:masterDec 26, 2019
@cjihrig
Copy link
ContributorAuthor

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

@cjihrigcjihrig deleted the wasi-err branch December 26, 2019 03:28
BridgeAR pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BridgeAR pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BridgeAR pushed 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: #31076Fixes: #30878 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@BridgeARBridgeAR mentioned this pull request Jan 7, 2020
targos pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed 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: #31076Fixes: #30878 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed 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: #31076Fixes: #30878 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@targostargos mentioned this pull request Jan 15, 2020
BethGriggs pushed a commit that referenced this pull request Feb 6, 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 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: #31076Fixes: #30878 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Feb 8, 2020
mhdawson added a commit to mhdawson/io.js that referenced this pull request Sep 25, 2023
- add check for case when trying to provide a better Exception fails - the code was modified to avoid a CHECK_EQ in all cases in nodejs#31076, however, I believe that if we fail to create the exeption to throw instead of simply returning using a CHECK makes more sense. I think it should also address the coverity warning about not initializing in the constructor. Signed-off-by: Michael Dawson <[email protected]>
mhdawson added a commit that referenced this pull request Oct 12, 2023
- add check for case when trying to provide a better Exception fails - the code was modified to avoid a CHECK_EQ in all cases in #31076, however, I believe that if we fail to create the exeption to throw instead of simply returning using a CHECK makes more sense. I think it should also address the coverity warning about not initializing in the constructor. Signed-off-by: Michael Dawson <[email protected]> PR-URL: #49866 Reviewed-By: Colin Ihrig <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
- add check for case when trying to provide a better Exception fails - the code was modified to avoid a CHECK_EQ in all cases in nodejs#31076, however, I believe that if we fail to create the exeption to throw instead of simply returning using a CHECK makes more sense. I think it should also address the coverity warning about not initializing in the constructor. Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#49866 Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
- add check for case when trying to provide a better Exception fails - the code was modified to avoid a CHECK_EQ in all cases in #31076, however, I believe that if we fail to create the exeption to throw instead of simply returning using a CHECK makes more sense. I think it should also address the coverity warning about not initializing in the constructor. Signed-off-by: Michael Dawson <[email protected]> PR-URL: #49866 Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs 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++.wasiIssues and PRs related to the WebAssembly System Interface.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

better error message for wasi

8 participants

@cjihrig@nodejs-github-bot@jasnell@Trott@addaleax@gengjiawen@legendecas@BridgeAR