[CXX-1217] GridFS download streams should kill the cursor when closed Created: 03/Feb/17 Updated: 17/Nov/17 Resolved: 10/Mar/17 |
|
| Status: | Closed |
| Project: | C++ Driver |
| Component/s: | API |
| Affects Version/s: | None |
| Fix Version/s: | 3.2.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Samuel Rossi (Inactive) | Assignee: | Samuel Rossi (Inactive) |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Description |
|
In the implementation of CXX-1130, the idea came up to kill the cursor being used to fetch the chunk when a download stream is closed. It isn't apparent what the most idiomatic way to do this would be; two options would be to either wrap the cursor in a unique pointer and then release it or to implement a method like `cursor::kill` specifically for these types of use cases. |
| Comments |
| Comment by Githook User [ 10/Mar/17 ] |
|
Author: {u'username': u'saghm', u'name': u'Saghm Rossi', u'email': u'saghmrossi@gmail.com'}Message: |
| Comment by Samuel Rossi (Inactive) [ 03/Mar/17 ] |
|
Per offline discussion, the fix will be to assign the null option to the cursor field in downloader::close |
| Comment by David Golden [ 04/Feb/17 ] |
|
I've retitled this about GridFS and scheduled it for this next release. We can get to it after the current work is merged. I don't want to extend that code review with any additional work. |
| Comment by J Rassi [ 03/Feb/17 ] |
|
Yep, indeed: the std::unique_ptr destructor (with the default deleter) calls delete on the managed pointer, whereas the stdx::optional destructor just invokes the destructor on the managed object. So, are we decided on scrapping the idea of adding cursor::kill(), in favor of making users use a wrapping type if they need to dynamically manage the cursor's lifetime? If yes, is there any work to do in this ticket? |
| Comment by David Golden [ 03/Feb/17 ] |
|
I prefer to still with the RAII approach. I'm not entirely clear whether an optional or a unique_ptr is better, but from what I understand, optional includes the memory allocation whereas unique_ptr does a new heap allocation, so we'd prefer to use optional and assign nullopt to it to clear it (thus killing the cursor). |
| Comment by J Rassi [ 03/Feb/17 ] |
|
Generally, letting a mongocxx::cursor object go out of scope is the correct way to initiate a kill cursor operation against the underlying server cursor (unless the cursor is exhausted, in which case the kill cursor operation is skipped). If you need to dynamically control the lifetime of a mongocxx::cursor object, then using a mongocxx::stdx::optional<mongocxx::cursor> object seems like the correct way to go, to me. Do you think that providing a second way to kill a cursor satisfies additional use cases / wouldn't just be unnecessarily adding additional driver logic? |