[SERVER-36637] IDL objects should hold owned BSONObjs Created: 14/Aug/18  Updated: 29/Oct/23  Resolved: 14/Sep/18

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

Type: Bug Priority: Major - P3
Reporter: Kyle Suarez Assignee: Mark Benvenuto
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by SERVER-36187 Use the IDL to serialize $out Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Platforms 2018-09-24
Participants:
Linked BF Score: 0

 Description   

When parsing BSONType::Object elements, the automatically-generated parse method will store a view of BSON in the spec, rather than an owned copy. For example,

document_source_out_gen.cpp

121
        else if (fieldName == kUniqueKeyFieldName) {
122
            if (MONGO_unlikely(usedFields[kUniqueKeyBit])) {
123
                ctxt.throwDuplicateField(element);
124
            }
125
 
126
            usedFields.set(kUniqueKeyBit);
127
 
128
            if (MONGO_likely(ctxt.checkAndAssertType(element, Object))) {
129
                _uniqueKey = element.Obj();
130
            }
131
        }

The spec then contains garbage if the original object used for parsing goes out-of-scope. Since the IDL already holds other owned types (e.g. std::string), we should strive to keep an owned copy of BSON.

I don't think this affects existing users of the IDL, as in most usages the original command object is kept around for the lifetime of the command's execution. However, we'd like to serialize $out with the IDL in SERVER-36187, but our reparse-reserialization tests hit a segmentation violation.



 Comments   
Comment by Mark Benvenuto [ 14/Sep/18 ]

Actually I tried to make the change but it broke the build.

Repl depends on the current behavior - https://github.com/mongodb/mongo/blob/082ece16ab827e4fbbc714a7bfafd5135e2bbf05/src/mongo/db/repl/rs_rollback.cpp#L571

I will leave it as is rather then causing unnecessary copies in repl.

Comment by Githook User [ 14/Sep/18 ]

Author:

{'name': 'Mark Benvenuto', 'email': 'mark.benvenuto@mongodb.com', 'username': 'markbenvenuto'}

Message: Revert "SERVER-36637 IDL objects should hold owned BSONObjs"

This reverts commit 846a6c19839601ce66f27877b348a4a5150a453d.
Branch: master
https://github.com/mongodb/mongo/commit/6f0a463deaf890219b4b08172ebaef28df81b470

Comment by Githook User [ 14/Sep/18 ]

Author:

{'name': 'Mark Benvenuto', 'email': 'mark.benvenuto@mongodb.com', 'username': 'markbenvenuto'}

Message: SERVER-36637 IDL objects should hold owned BSONObjs
Branch: master
https://github.com/mongodb/mongo/commit/846a6c19839601ce66f27877b348a4a5150a453d

Comment by Kyle Suarez [ 20/Aug/18 ]

For the sake of completeness, $mergeCursors works around this by keeping the original BSONObj in scope for the lifetime of its execution. That's an alternative approach we could take if we didn't want to change the behavior of the IDL.

Comment by Kyle Suarez [ 14/Aug/18 ]

Okay, after a brief attempt at this, I don't think it's worth the effort of me trying to finish this. At the moment, SERVER-36187 is not on the critical path for the $out project, so I'm dropping this into the Platforms Team needs triage queue to focus on required pieces of the $out project.

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