Ticket #219 (new defect)

Opened 2 years ago

Last modified 7 months ago

napi error reporting not thread safe

Reported by: Tobias Richter Owned by: Unassigned
Priority: major Milestone:
Component: napi Version: 4.2.0
Keywords: Cc:

Description

NXIReportError and NXpData are held as global variables in napi.c. So changing the error reporting (NXMSetError) in one thread affects all threads.

We see occasional crashes of the JVM. The Java binding stores thread information in NXpData. If that is overwritten in another thread and an error occurs in the first thread the JNI code tries to throw an exception in the wrong thread. At least I think this is what is happening. Now that I have come up with that theory I can more easily test it.

But I am pretty sure the implementation is broken (for all bindings using napi.c). Please correct me if I am wrong.

Change History

comment:1 Changed 2 years ago by Tobias Richter

(In [1423]) refs #210 #219

work around threading issue in c api by getting environment dynamically simplify error reporting

comment:2 Changed 16 months ago by Tobias Richter

(In [1542]) allow setting of error reporting in thread local storage so that multiple threads can have their own error reporting refs #219

comment:3 Changed 16 months ago by Tobias Richter

(In [1543]) use threaded version of error handling setters in Java refs #219

comment:4 Changed 16 months ago by Tobias Richter

(In [1546]) use Disable and Enable ErrorReporting so that threaded versions are respected refs #219

comment:5 Changed 16 months ago by Tobias Richter

(In [1548]) create new error reporting function that only takes a message as parameter and use that throughout the api refs #219

comment:6 Changed 16 months ago by Tobias Richter

(In [1552]) Error reporting may have failed due to memory corruption or whatever but was masked by threading problems. If we get a fresh class reference every time we want to throw an error that helps. refs #219

comment:7 Changed 16 months ago by Tobias Richter

(In [1553]) check for thread local storage MacOS X doesn't have it (this way) refs #219

comment:8 Changed 16 months ago by Tobias Richter

(In [1556]) remove DEBUG define left in there refs #219

comment:9 Changed 16 months ago by Tobias Richter

(In [1557]) fix backwards logic the issued the waring when everything was fine refs #219

comment:10 Changed 16 months ago by Tobias Richter

(In [1562]) for additional safety bring back the detection of jvm refs #219

comment:11 Changed 16 months ago by Tobias Richter

(In [1567]) changes to fix the threading problems in the error reporting refs #219

comment:12 Changed 7 months ago by Tobias Richter

(In [1598]) changes to fix the threading problems in the error reporting refs #219

Note: See TracTickets for help on using tickets.