[CXX-974] Wrapping mongocxx::client Created: 20/Jul/16  Updated: 11/Sep/19  Resolved: 20/Jul/16

Status: Closed
Project: C++ Driver
Component/s: API
Affects Version/s: 3.0.0
Fix Version/s: None

Type: Task Priority: Critical - P2
Reporter: Artur Shaykhutdinov [X] Assignee: Unassigned
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

ubuntu 16.04 lts, gcc 5.4.0



 Description   

I want to wrap mongocxx::client in some class.
I wrote this class:

MongoDbConnection.h

 
class MongoDbConnection
{
public:
    explicit MongoDbConnection(const std::string& dbName) :
        dbName(dbName)
      ,instance()
      ,connection()
    {
    }
    ~MongoDbConnection()
    {
    }
 
    void connect(const std::string& host)
    {
        if (connection)
        {
            return;
        }
        
        mongocxx::uri uri(host);
        connection = std::move(mongocxx::client(uri));
    }
    DatabaseCursor findWithCursor(const std::string& collectionName, const view_or_value& query)
    {
        if (isConnectionOk())
        {
            mongocxx::collection collection = connection
                    .database(dbName)
                    .collection(collectionName);
            mongocxx::cursor cursor = collection.find(query);
            return DatabaseCursor(cursor);
        }
        DatabaseCursor dbCursor;
        return dbCursor;
    }
 
private:
    bool isConnectionOk() const
    {
        const bool res = (bool)connection;
        return res;
    }
 
    const std::string dbName;
    mongocxx::instance instance;
    mongocxx::client connection;
};

And class which contains mongocxx::cursor

DatabaseCursor.h

class DatabaseCursor
{
public:
    DatabaseCursor() :
        current(nullptr)
      ,end(nullptr)
      ,_isNull(true)
    {
        std::cout << "DatabaseCursor::DatabaseCursor() :" << std::endl;
    }
    DatabaseCursor(mongocxx::cursor& cursor) :
        current(new mongocxx::cursor::iterator(cursor.begin()))
      ,end(new mongocxx::cursor::iterator(cursor.end()))
      ,_isNull(current == end)
    {
        std::cout << "DatabaseCursor::DatabaseCursor(mongocxx::cursor& cursor) :"  << std::endl;
    }
    DatabaseCursor(DatabaseCursor&& cursor) :
        current(std::move(cursor.current))
      ,end(std::move(cursor.end))
      ,_isNull(std::move(cursor._isNull))
    {
        std::cout << "DatabaseCursor::DatabaseCursor(DatabaseCursor&& cursor) :"  << std::endl;
    }
 
    DatabaseCursor(const DatabaseCursor& other) = delete;
    DatabaseCursor& operator =(const DatabaseCursor& other) = delete;
 
    bool next(bsoncxx::document::value& obj)
    {
        if (!current || !end || (*current) == (*end))
        {
            return false;
        }
        mongocxx::cursor::iterator iter = (*current);
        bsoncxx::document::view_or_value doc(*iter);
        obj = bsoncxx::document::value(doc.view());
        //    ++(*current);
        try
        {
            ++iter;
        }
        catch (mongocxx::query_exception& e)
        {
            std::cout << e.what()  << std::endl;
        }
 
        current.reset(new mongocxx::cursor::iterator(iter));
 
        return true;
    }
    bool isNull() const
    {
        return _isNull;
    }
 
private:
    std::unique_ptr<mongocxx::cursor::iterator> current, end;
    bool _isNull;
};



 Comments   
Comment by David Golden [ 20/Jul/16 ]

Hi, Artur. The problem is that the iterators are outlasting the lifetime of the cursor. You need to hold onto the cursor and derive your iterators from the saved copy.

I implemented your original code as a single file and confirmed the segfault. Then I wrote a revised version that saves the cursor. This is the diff:

--- orig.cpp	2016-07-20 12:55:34.000000000 -0400
+++ revised.cpp	2016-07-20 13:39:34.000000000 -0400
@@ -3,6 +3,7 @@
 #include <vector>
 
 #include <bsoncxx/json.hpp>
+#include <bsoncxx/stdx/make_unique.hpp>
 
 #include <mongocxx/client.hpp>
 #include <mongocxx/instance.hpp>
@@ -10,21 +11,24 @@
 #include <mongocxx/uri.hpp>
 #include <mongocxx/exception/query_exception.hpp>
 
+using bsoncxx::stdx::make_unique;
+
 class DatabaseCursor {
 public:
   DatabaseCursor() : current(nullptr), end(nullptr), _isNull(true) {
     std::cout << "DatabaseCursor::DatabaseCursor() :" << std::endl;
   }
-  DatabaseCursor(mongocxx::cursor &cursor)
-      : current(new mongocxx::cursor::iterator(cursor.begin())),
-        end(new mongocxx::cursor::iterator(cursor.end())),
+  DatabaseCursor(mongocxx::cursor &&cursor)
+      : _cursor(make_unique<mongocxx::cursor>(std::move(cursor))),
+        current(make_unique<mongocxx::cursor::iterator>(_cursor->begin())),
+        end(make_unique<mongocxx::cursor::iterator>(_cursor->end())),
         _isNull(current == end) {
-    std::cout << "DatabaseCursor::DatabaseCursor(mongocxx::cursor& cursor) :"
+    std::cout << "DatabaseCursor::DatabaseCursor(mongocxx::cursor&& cursor) :"
               << std::endl;
   }
   DatabaseCursor(DatabaseCursor &&cursor)
-      : current(std::move(cursor.current)), end(std::move(cursor.end)),
-        _isNull(std::move(cursor._isNull)) {
+      : _cursor(std::move(cursor._cursor)), current(std::move(cursor.current)),
+        end(std::move(cursor.end)), _isNull(std::move(cursor._isNull)) {
     std::cout << "DatabaseCursor::DatabaseCursor(DatabaseCursor&& cursor) :"
               << std::endl;
   }
@@ -53,6 +57,7 @@
   bool isNull() const { return _isNull; }
 
 private:
+  std::unique_ptr<mongocxx::cursor> _cursor;
   std::unique_ptr<mongocxx::cursor::iterator> current, end;
   bool _isNull;
 };
@@ -69,7 +74,7 @@
     }
 
     mongocxx::uri uri(host);
-    connection = std::move(mongocxx::client(uri));
+    connection = mongocxx::client(uri);
   }
   DatabaseCursor findWithCursor(const std::string &collectionName,
                                 const bsoncxx::document::view_or_value &query) {
@@ -77,7 +82,7 @@
       mongocxx::collection collection =
           connection.database(dbName).collection(collectionName);
       mongocxx::cursor cursor = collection.find(query);
-      return DatabaseCursor(cursor);
+      return DatabaseCursor(std::move(cursor));
     }
     DatabaseCursor dbCursor;
     return dbCursor;

Note: I also removed an unnecessary std::move assigning to connection (as reported by -Wall).

I'm going to close this ticket, but feel free to reopen if you have additional questions.

Comment by Artur Shaykhutdinov [X] [ 20/Jul/16 ]

int main(int, char**)
{
    MongoDbConnection dbConnection("db_test");
    dbConnection.connect("mongodb://localhost:27017");
    bsoncxx::document::value query = document{} << finalize;
    DatabaseCursor dbCursor(dbConnection.findWithCursor("collection_name", query.view()));
 
    bsoncxx::document::value obj = bsoncxx::builder::stream::document{} << finalize;
    while (dbCursor.next(obj))
    {
        std::cout << bsoncxx::to_json(obj) << std::endl << std::endl;
    }
 
    return 0;
}

When I use these classes, the program crashed with "segmentation fault" at line with code

++iter

in DatabaseCursor.h

Generated at Wed Feb 07 22:00:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.