[SERVER-66389] segfault due to null JsFunction pointer for auto-parameterized $where Created: 11/May/22  Updated: 07/Dec/23  Resolved: 13/May/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 6.0.0-rc5, 6.1.0-rc0
Fix Version/s: 6.0.0-rc6, 6.1.0-rc0

Type: Bug Priority: Critical - P2
Reporter: David Storch Assignee: David Storch
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Related
related to SERVER-83959 When preparing SBE plan, correctly pa... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v6.0
Sprint: QE 2022-05-16
Participants:
Linked BF Score: 123

 Description   

When binding the value associated with an auto-parameterized $where MatchExpression node, we have a performance optimization to transfer ownership of the JsFunction rather than copying it. See the code here. The motivation for this optimization is that copying a JsFunction is very expensive, as it requires some kind of expensive initialization of the JavaScript engine.

This optimization is not correct if there are multiple candidate plans. Imagine a scenario where there are two candidate plans. During preparation of the first candidate, we need to bind the JsFunction into the SBE runtime environment. The optimization kicks in, so we extract the JsFunction from the CanonicalQuery and pass ownership to the runtime environment. This leaves a nullptr in the $where node. For the second candidate plan, the optimization kicks in again. This time, the JsFunction pointer is null. The ownership transfer logic tolerates nullptr and is essentially a no-op this time around.

Later, when we actually try to execute the second candidate plan's trial period, we assume that the JsFunction pointer is non-null and attempt to dereference it, resulting in a segmentation fault. This bug affects both master and the 6.0 branch.

The easy way to fix this would be to revert the to optimization, and just copy the JsFunction into each candidate plan. (The implementation is a bit sketchy as is anyway, since it uses const_cast to discard the const qualifier.) The downside is that this approach have negative performance consequences. Another option would be to copy the JsFunction into all of the candidate plans except for the last one, or possibly to only have the optimization kick in when there is just one candidate plans. One other idea to look into is that we do clone the MatchExpression tree during plan enumeration – perhaps the JsFunction objects associated with each copy can be transferred to the corresponding SBE plans?



 Comments   
Comment by Githook User [ 13/May/22 ]

Author:

{'name': 'David Storch', 'email': 'david.storch@mongodb.com', 'username': 'dstorch'}

Message: SERVER-66389 Fix $where parameter bind-in optimization

(cherry picked from commit 48acc21bc952810c7028f79773905f9cdcce44af)
Branch: v6.0
https://github.com/mongodb/mongo/commit/94ebb3709d96e4d991294c0c1db021f83d9b2c58

Comment by Githook User [ 13/May/22 ]

Author:

{'name': 'David Storch', 'email': 'david.storch@mongodb.com', 'username': 'dstorch'}

Message: SERVER-66389 Fix $where parameter bind-in optimization
Branch: master
https://github.com/mongodb/mongo/commit/48acc21bc952810c7028f79773905f9cdcce44af

Comment by David Storch [ 12/May/22 ]

I am removing this from the "6.0 Required" bucket due to SERVER-66445, though I still intend to merge it shortly. (At the moment it is conflicting with other patches in the commit queue due to the changes in etc/backports_required_for_multiversion_tests.yml so I need to wait for the queue to drain.)

Generated at Thu Feb 08 06:05:18 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.