[SERVER-29666] Interrupt signal should cause localConnectForDbEval to fail Created: 15/Jun/17  Updated: 27/Oct/23  Resolved: 27/Oct/17

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

Type: Bug Priority: Trivial - P5
Reporter: Spencer Jackson Assignee: Spencer Jackson
Resolution: Gone away Votes: 0
Labels: neweng, platforms-interns-2017
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related
is related to SERVER-29651 Interrupt signal is ignored when rece... Closed
Operating System: ALL
Participants:
Linked BF Score: 0

 Description   

localConnectForDbEval makes multiple calls to 'exec', without checking whether they succeeded or failed. If an interrupt is triggered, the JS engine will cleanup the scope, causing one of the exec calls to fail. The next call to exec may access cleaned up resources.

A failing invokation of exec should cause the whole caller to fail.



 Comments   
Comment by Spencer Jackson [ 27/Oct/17 ]

ted.tuckman is correct. At the time of writing the exec functions in the file threw on errors, so they could not have been the JS invocations which caused the issue. A closer look at the logs reveals the following error right before the access:

[MongoDFixture:job0] 2017-04-19T15:37:29.682+0000 E - [js] unable to load stored JavaScript function foo(): 1 Failed to call method

MozJSImplScope calls into JS in order to parse a function's AST. "Unable to load stored JavaScript function" comes from loading system.js, the "Failed to call method" happens when Scope::loadStored calls into the chain MozJSImplScope::setElement() > ObjectWrapper::setBSONElement -> ValueReader::fromBSONElement scope>newFunction() > _MozJSCreateFunction() -> parseJSFunctionOrExpression> ObjectWrapper::callMethod(). callMethod probably observed the interrupt which occured a few milliseconds before the error.

The changes in SERVER-29651 wrapped MozJSImplScope::setElement in a _runSafely call, so this issue should have gone away.

Comment by Ted Tuckman [ 26/Jul/17 ]

It seems like this has gone away. In localConnectForDbEval, we call exec with assertOnError = true. This causes exec to throw if it has a not OK status when it calls _checkErrorState. In the linked ticket, localConnect was wrapped with a function that should handle exceptions correctly and pass them on to the calling function. If exec fails it should throw and be handled now, instead of running the second exec. It is also unclear to me if the second exec would have been called before this change, seeing as an exception would have been thrown if the first one fails. If the status is not set by a failing exec and it sends a Status::OK to _checkErrorState regardless of success/failure, then it would continue and would need another solution.

Generated at Thu Feb 08 04:21:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.