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.

Sign up for GitHub

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

Jump to bottom

JS error when NaCl crashes and there's active client #823

Closed
emaxx-google opened this issue Jul 18, 2023 · 1 comment · Fixed by #824
Closed

JS error when NaCl crashes and there's active client #823

emaxx-google opened this issue Jul 18, 2023 · 1 comment · Fixed by #824
Assignees
@ViktoriiaKovalova

Comments

@emaxx-google
Copy link
Collaborator

Just observed this on a test device after the Connector's NaCl module crashed (for an unrelated reason):

Uncaught TypeError: Cannot read properties of null (reading 'g')
    at I (background.js:101:481)
    at kl.A (background.js:456:49)
    at I (background.js:101:494)
    at background.js:457:296
    at H.A (background.js:101:911)
    at fl.A (background.js:451:41)
    at I (background.js:101:494)
    at background.js:450:325
    at H.A (background.js:101:911)
    at h.A (background.js:178:175)

The stack trace is obfuscated due to minification, but after comparing the minified and original code I think it's crashing in ClientHandler.disposeInternal():

chromeos_smart_card_connector/third_party/pcsc-lite/naclport/server_clients_management/src/client-handler.js

Line 211 in f4eb322

this.serverRequester_.dispose();

@emaxx-google
Copy link
Collaborator Author

Suspecting this to be unintended side effect of the refactoring in #794. Viktoriia, could you PTAL? Thanks.

@ViktoriiaKovalova ViktoriiaKovalova mentioned this issue Jul 18, 2023
Merged
This was referenced Jul 18, 2023
Merged
Merged
Merged
Merged
Merged
Merged
ViktoriiaKovalova added a commit that referenced this issue Jul 21, 2023
@ViktoriiaKovalova
Fix dispose called on null during ClientHandler disposal (#824)
48738ec 
This fixes #823.
emaxx-google added a commit that referenced this issue Sep 8, 2023
@emaxx-google
[JS-to-C++ test] Boilerplate for //common/js tests
4f8f1a0 
Add build target for //common/js/build/js_to_cxx_tests/, so that we can
write tests for this common JS and C++ code.

This is preparation for implementing a regression test for #823.
@emaxx-google emaxx-google mentioned this issue Sep 8, 2023
Merged
emaxx-google added a commit that referenced this issue Sep 8, 2023
@emaxx-google
[JS-to-C++ test] Boilerplate for //common/js tests (#1029)
27e06f1 
Add build target for //common/js/build/js_to_cxx_tests/, so that we can
write tests for this common JS and C++ code.

This is preparation for implementing a regression test for #823.
@emaxx-google emaxx-google mentioned this issue Sep 12, 2023
Merged
emaxx-google added a commit that referenced this issue Sep 12, 2023
@emaxx-google
JS Requester handles exceptions correctly (#1048)
5be8254 
If sending a message failed, the Requester.postRequest() message should
reject the returned promise instead of failing itself with an exception.

A real example when this can happen is when sending a message that
causes a crash in an Emscripten module, because in that case the
exception is bubbled up synchronously.

This is preparation for implementing a regression test for #823.
emaxx-google added a commit that referenced this issue Sep 13, 2023
@emaxx-google
[JS-to-C++ test] Test crashes
370aa87 
Add a test that crash in a C++ executable module gets correctly handled
on the JS side, in particular marks the module as disposed.

This test verifies a slightly simpler scenario than the regression fixed
in #823.
@emaxx-google emaxx-google mentioned this issue Sep 13, 2023
Merged
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
[JS-to-C++ test] Test crashes
50cfbf5 
Add a test that crash in a C++ executable module gets correctly handled
on the JS side, in particular marks the module as disposed.

This test verifies a slightly simpler scenario than the regression fixed
in #823.
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
[JS-to-C++ test] Test crashes (#1053)
ecb0da8 
Add a test that crash in a C++ executable module gets correctly handled
on the JS side, in particular marks the module as disposed.

This test verifies a slightly simpler scenario than the regression fixed
in #823.
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
JS Requester handles disposal-and-exception-while-sending
6bdacfb 
Improve the JS Requester class to gracefully handle this edge case:
the send() call triggers not only an exception, but also the disposal
of the message channel. The bug was that we were trying to reject the
same request twice (first during disposal, second while handling the
exception).

This is follow-up to the crash handling issue fixed in #823.
@emaxx-google emaxx-google mentioned this issue Sep 14, 2023
Merged
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
[JS-to-C++ test] Test crash with active PC/SC client
99c3f88 
Test that a C++ crash in the executable module gets correctly
handled in case there's an active PC/SC client. The module and the
client should get disposed of without any unexpected exceptions.

This is a regression test for #823.
@emaxx-google emaxx-google mentioned this issue Sep 14, 2023
Merged
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
[JS-to-C++ test] Test crash with active PC/SC client
4813edb 
Test that a C++ crash in the executable module gets correctly
handled in case there's an active PC/SC client. The module and the
client should get disposed of without any unexpected exceptions.

This is a regression test for #823.
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
JS Requester handles disposal-and-exception-while-sending (#1057)
f5b01bb 
Improve the JS Requester class to gracefully handle this edge case:
the send() call triggers not only an exception, but also the disposal
of the message channel. The bug was that we were trying to reject the
same request twice (first during disposal, second while handling the
exception).

This is follow-up to the crash handling issue fixed in #823.
emaxx-google added a commit that referenced this issue Sep 14, 2023
@emaxx-google
[JS-to-C++ test] Test crash with active PC/SC client
0a8fbfa 
Test that a C++ crash in the executable module gets correctly
handled in case there's an active PC/SC client. The module and the
client should get disposed of without any unexpected exceptions.

This is a regression test for #823.
emaxx-google added a commit that referenced this issue Sep 15, 2023
@emaxx-google
[JS-to-C++ test] Test crash with active PC/SC client (#1058)
00e26da 
Test that a C++ crash in the executable module gets correctly
handled in case there's an active PC/SC client. The module and the
client should get disposed of without any unexpected exceptions.

This is a regression test for #823.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ViktoriiaKovalova ViktoriiaKovalova

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@emaxx-google @ViktoriiaKovalova