[CXX-1184] Prevent misuse of client temporaries Created: 05/Jan/17 Updated: 27/Mar/18 Resolved: 08/Mar/18 |
|
| Status: | Closed |
| Project: | C++ Driver |
| Component/s: | Documentation |
| Affects Version/s: | None |
| Fix Version/s: | 3.3.0-rc0 |
| Type: | New Feature | Priority: | Major - P3 |
| Reporter: | David Golden | Assignee: | Unassigned |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Description |
|
(Original title: "find/cursor documentation doesn't warn about client lifetime") From https://groups.google.com/forum/#!topic/mongodb-user/girvbR21AzM:
|
| Comments |
| Comment by Jamie Lannister [ 27/Mar/18 ] | |||||||||||||||||||||||||||
|
I'm sorry, Please ignore my previous comments. I think I have fixed my problem. the issue is that I have a main project which includes another sub project with git submodule. within both the main project and the sub project, there are MongoDb wrappers defined using the same name. This must have confused the compiler/linker and caused some thing uninitialized. I have renamed one of the wrapper to use a different name and the issue seemed to be fixed. | |||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 27/Mar/18 ] | |||||||||||||||||||||||||||
|
It is impossible to define copy or copy-assignment operators for that class exactly because it contains a unique_ptr. Similarly, the compiler generated move constructor and move assignment operator should be automatically correct. I suspect something else is going wrong. As this ticket is closed, I'd prefer that you do one of the following so we can continue to investigate:
That will make it easier for us to assign someone to work with you to understand the root cause of this issue. | |||||||||||||||||||||||||||
| Comment by Jamie Lannister [ 27/Mar/18 ] | |||||||||||||||||||||||||||
|
it is a crash, a segmentation fault caused by assignment of the object mongocxx::pool::entry the issue is because of the unique pointer used in that class: should a copy constructor and a operator= be defined for this class?
| |||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 27/Mar/18 ] | |||||||||||||||||||||||||||
|
serjaimelannister - What is the error message your compiler emits? | |||||||||||||||||||||||||||
| Comment by Jamie Lannister [ 27/Mar/18 ] | |||||||||||||||||||||||||||
|
The change for this bug, commit https://github.com/mongodb/mongo-cxx-driver/commit/423d3b9613c5fa0c95d649f528c064107df86ef0 seems to break my code. In my code, I have a mongodb wrapper class, in which there is a getClient() that returns a client to use. the function is defined:
the client, as returned by pool.acquire() is not returnable because of the unique_ptr member. | |||||||||||||||||||||||||||
| Comment by Githook User [ 17/Mar/18 ] | |||||||||||||||||||||||||||
|
Author: {'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis', 'username': 'ajdavis'}Message: | |||||||||||||||||||||||||||
| Comment by Githook User [ 08/Mar/18 ] | |||||||||||||||||||||||||||
|
Author: {'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis', 'username': 'ajdavis'}Message: | |||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 24/Feb/18 ] | |||||||||||||||||||||||||||
|
Demo (this segfaults):
Swapping the line tmp = nullptr; with the next line allows the program to complete. | |||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 05/Jan/17 ] | |||||||||||||||||||||||||||
|
My point was more that the expression as written contains two bugs, not just one. As for lifecycle contracts, this was extensively debated during the design phase of the C++11 driver, and the conclusion was that we shouldn't try to guarantee them - C++ isn't Java, and managing lifetimes to honor contracts is an expected part of using the language and its associated libraries. What we can, and should, do, is document those constraints and attempt to make apparently safe idioms not compile. We can do that for above case by disallowing invocation of 'find' or other cursor returning objects on collection temporaries. Note also that it is almost certainly possible to build a set of shared pointer managed types that sit above client/database/collection/cursor and do the right thing. | |||||||||||||||||||||||||||
| Comment by David Golden [ 05/Jan/17 ] | |||||||||||||||||||||||||||
|
Note: per Slack conversation, mongoc_cursor_t only really depends on mongoc_client_t. We should wait until | |||||||||||||||||||||||||||
| Comment by David Golden [ 05/Jan/17 ] | |||||||||||||||||||||||||||
|
From http://mongoc.org/libmongoc/current/mongoc_collection_t.html:
From http://mongoc.org/libmongoc/current/mongoc_database_t.html
From http://mongoc.org/libmongoc/current/mongoc_client_t.html:
We need to think more broadly how to guarantee those lifecycle contracts because right now we don't guarantee the destruction order that libmongoc mandates. While I know we've avoided shared pointers, having everything reference count upwards would certainly do the trick. That has a cost, sure, but I'm not sure how else we can satisfy the contract in code. We could document the same constraints for our users, but then we're sort of doing RAII in name only, as they still need to manage destruction order manually. Other ideas? | |||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 05/Jan/17 ] | |||||||||||||||||||||||||||
|
Is it legal to use the cursor result of mongocxx::collection::find after the mongocxx::collection has been destroyed? If not, there may actually be two bugs in the above statement, but it is not clear from the underlying mongoc docs whether or not it is OK. Independently of that, it may in fact be possible to make the above bug illegal, but returning a proxy object rather than a bare unique_ptr from mongocxx::pool::acquire. The proxy object would implement operator-> but it would have an explicitly deleted operator-> for the && case, much like mongocxx::client::database. |