[SERVER-5842] Exceptions thrown in scripting/engine_spidermonkey.cpp need to be handled there Created: 14/May/12  Updated: 03/Jan/18  Resolved: 30/May/12

Status: Closed
Project: Core Server
Component/s: Shell
Affects Version/s: None
Fix Version/s: 2.1.2

Type: Bug Priority: Major - P3
Reporter: Tad Marshall Assignee: Tad Marshall
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by SERVER-5796 HexData undocumented, crashy Closed
Duplicate
is duplicated by SERVER-3060 db.artworks.findOne("genome.genes.Col... Closed
is duplicated by SERVER-4913 prevent process segv crash in javascr... Closed
is duplicated by SERVER-5482 array access attempt after a find fai... Closed
Related
related to SERVER-5967 unhandled exception: boost::filesyste... Closed
related to SERVER-1613 shell abends on bad UTF8 Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Participants:
Case:

 Description   

There are a few cases where a "native" JavaScript function implemented in scripting/engine_spidermonkey.cpp or scripting/sm_db.cpp will throw an exception. An example (currently) is the native_print() function which calls Convertor::toString() which can throw an exception. If this exception isn't caught before control passes out through SpiderMonkey then SpiderMonkey is left in a corrupted state and attempts to use JavaScript after this, even to display the shell's prompt, will generate access violations (segfaults).

We need to use try/catch around any exceptions that can be thrown within engine_spidermonkey.cpp and return normally (usually with JS_FALSE) so that SpiderMonkey can maintain its state properly.



 Comments   
Comment by auto [ 31/May/12 ]

Author:

{u'login': u'tadmarshall', u'name': u'Tad Marshall', u'email': u'tad@10gen.com'}

Message: SERVER-5842 don't assert on call inside native_helper

The native_helper() routine in engine_spidermonkey.cpp calls
functions that use Boost, and Boost will throw std:exceptions
on cases such as file and directory protection errors, badly
formed filenames, and others, so we can't call fassertFailed
on these exceptions; they are user errors and need to be passed
back to the JavaScript caller.
Branch: master
https://github.com/mongodb/mongo/commit/d0e8e4f665c69e2ee384c0828457038d40b1498e

Comment by auto [ 30/May/12 ]

Author:

{u'login': u'tadmarshall', u'name': u'Tad Marshall', u'email': u'tad@10gen.com'}

Message: SERVER-5842 – catch SocketExceptions, handle as non-fatal

For selected functions, catch SocketExceptions and pass them
to JavaScript instead of calling fassertFailed. Starting with
a small set of functions, we may need to add others later.
Branch: master
https://github.com/mongodb/mongo/commit/d54bc803f9226b57a7eb58f07f856beb361fb658

Comment by auto [ 30/May/12 ]

Author:

{u'login': u'tadmarshall', u'name': u'Tad Marshall', u'email': u'tad@10gen.com'}

Message: SERVER-5842 don't let exceptions leave engine_spidermonkey.cpp

In every routine called by SpiderMonkey, use try/catch to grab all
AssertionException C++ exceptions and translate them into JavaScript
exceptions. Catch all std::exceptions, log them and then fassert.
Some error handling was made a bit more user friendly, e.g. showing
correct syntax instead of saying "invalid syntax". Some C-style
casting was changed to static_cast or reinterpret_cast. Some
JSClass structures were formatted for readability. Some long lines
were reformatted. The order of some tests were changed so that we
test for invalid arguments before we start allocating memory. Some
existing try/catch code was changed to use a single try/catch over
the entire routine, but others were made nested in order to
preserve existing error messages that may be important to Buildbot
tests.
Branch: master
https://github.com/mongodb/mongo/commit/72e14b4afbb2777281eb0c40612596ed1223730e

Comment by auto [ 27/May/12 ]

Author:

{u'login': u'tadmarshall', u'name': u'Tad Marshall', u'email': u'tad@10gen.com'}

Message: Code cleanup prior to fixing SERVER-5842

Make three injected native JavaScript routines static since they are
not referenced outside this file. Name them according to the convention
in engine_spidermonkey.cpp. Use uassert consistently. Make (user) error
messages correct, consistent and informative.
Branch: master
https://github.com/mongodb/mongo/commit/0735336834dd9e55a62c6ce6baa0b1b525d7bd65

Comment by Tad Marshall [ 22/May/12 ]

After researching JavaScript exceptions in the SpiderMonkey code and online (Mozilla) documentation I learned that the JS_ReportError() function that we call from some places actually creates a JavaScript exception that can be caught with a JavaScript try/catch. This seems like the right way to handle all errors in JavaScript that are not fatal MongoDB conditions. Incorrect use of (for example) BSON objects created in JavaScript should be handled by sending a JavaScript exception to the code that caused it (by using JS_ReportError()) and should not be treated as MongoDB exceptions. Following this approach throughout engine_spidermonkey.cpp and sm_db.cpp should eliminate the crashing problems we've seen following JavaScript failures.

Comment by Tad Marshall [ 18/May/12 ]

Moving from 2.3.0 to 2.1.2 because I resolved SERVER-5482 (scheduled for 2.1.2) as a duplicate of this ticket. Any uassert, verify or throw inside of engine_spidermonkey.cpp or sm_db.cpp that isn't caught will make SpiderMonkey unusable.

Generated at Thu Feb 08 03:10:02 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.