[SERVER-25540] investigate why StorageInterfaceImpl::_findOrDeleteOne needs to make owned copy of BSON document returned from PlanExecutor Created: 10/Aug/16 Updated: 16/Aug/16 Resolved: 16/Aug/16 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Querying |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Benety Goh | Assignee: | David Storch |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Operating System: | ALL | ||||||||
| Sprint: | Query 2016-09-19 | ||||||||
| Participants: | |||||||||
| Linked BF Score: | 0 | ||||||||
| Comments |
| Comment by Benety Goh [ 16/Aug/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thanks for looking into this. We definitely want the BSONObj to outlive the plan executor, so getOwned() works for us. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by David Storch [ 16/Aug/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I spoke with benety.goh in person who confirmed that this is the desired fix. The returned BSONObj needs to outlive the PlanExecutor that generated it, and therefore it must always be owned. Closing as Works as Designed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by David Storch [ 15/Aug/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
charlie.swanson, got it, things are starting to make sense now. The IXSCAN stage is totally allowed to return an unowned BSONObj. It will remain unowned if there is no subsequent parent stage, such as DELETE, which might transition the BSONObj to owned. So it doesn't look like there is a problem where the DELETE stage with the returnDeleted flag can somehow manage to return unowned BSON. benety.goh, given this explanation, do you still believe that calling getOwned() as done in https://github.com/mongodb/mongo/commit/33f11f7786c4e742fe7a75333323612671667e32 is the correct fix? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 15/Aug/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
After some investigation, it looks like jstests/replsets/initial_sync_update_missing_doc1.js is causing this path to be executed during StorageInterfaceImpl::_findOrDeleteOne. This was determined after applying the following patch:
Generated the attached backtrace while running jstests/replsets/initial_sync_update_missing_doc1.js. |