DRAFT: Include a `_deletable` flag

include/ccaosdb.h

@@ -61,6 +61,7 @@ const char *caosdb_constants_COMPATIBLE_SERVER_VERSION_PRE_RELEASE();
  */
 typedef struct {
   void *wrapped_connection;
+  bool _deletable = false;
 } caosdb_connection_connection;
 
 /**
@@ -72,6 +73,7 @@ typedef struct {
  */
 typedef struct {
   void *wrapped_connection_configuration;
+  bool _deletable = false;
 } caosdb_connection_connection_configuration;
 
 /**
@@ -91,10 +93,12 @@ typedef struct {
 
 typedef struct {
   void *wrapped_certificate_provider;
+  bool _deletable = false;
 } caosdb_connection_certificate_provider;
 
 typedef struct {
   void *wrapped_authenticator;
+  bool _deletable = false;
 } caosdb_authentication_authenticator;
 
 /**
@@ -263,6 +267,7 @@ int caosdb_connection_connection_manager_get_connection(
 // not sufficient yet.
 typedef struct {
   void *wrapped_transaction;
+  bool _deletable = false;
 } caosdb_transaction_transaction;
 
 /**
@@ -289,6 +294,7 @@ int caosdb_transaction_transaction_execute(
 
 typedef struct {
   void *wrapped_result_set;
+  bool _deletable = false;
 } caosdb_transaction_result_set;
 
 int caosdb_transaction_transaction_get_result_set(
@@ -300,6 +306,7 @@ int caosdb_transaction_transaction_get_count_result(
 
 typedef struct {
   void *wrapped_entity;
+  bool _deletable = false;
 } caosdb_entity_entity;
 
 int caosdb_transaction_result_set_at(caosdb_transaction_result_set *result_set,
@@ -309,12 +316,15 @@ int caosdb_transaction_result_set_size(
 
 typedef struct {
   void *wrapped_property;
+  bool _deletable = false;
 } caosdb_entity_property;
 typedef struct {
   void *wrapped_parent;
+  bool _deletable = false;
 } caosdb_entity_parent;
 typedef struct {
   void *wrapped_message;
+  bool _deletable = false;
 } caosdb_entity_message;
 
 // GETTERS FOR EVERYTHING

src/ccaosdb.cpp

@@ -180,6 +180,7 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                     out->wrapped_certificate_provider =
                       new caosdb::configuration::PemFileCertificateProvider(
                         std::string(path));
+                    out->_deletable = true;
                     return 0;
                   })
 
@@ -188,8 +189,11 @@ ERROR_RETURN_CODE(
   int caosdb_connection_delete_certificate_provider(
     caosdb_connection_certificate_provider *provider),
   {
-    delete static_cast<caosdb::configuration::CertificateProvider *>(
-      provider->wrapped_certificate_provider);
+    if (provider->_deletable) {
+      delete static_cast<caosdb::configuration::CertificateProvider *>(
+        provider->wrapped_certificate_provider);
+    }
+    provider->_deletable = false;
     return 0;
   })
 
@@ -201,17 +205,22 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                     out->wrapped_authenticator =
                       new caosdb::authentication::PlainPasswordAuthenticator(
                         std::string(username), std::string(password));
+                    out->_deletable = true;
                     return 0;
                   })
 
-ERROR_RETURN_CODE(GENERIC_ERROR,
-                  int caosdb_authentication_delete_authenticator(
-                    caosdb_authentication_authenticator *authenticator),
-                  {
-                    delete static_cast<caosdb::authentication::Authenticator *>(
-                      authenticator->wrapped_authenticator);
-                    return 0;
-                  })
+ERROR_RETURN_CODE(
+  GENERIC_ERROR,
+  int caosdb_authentication_delete_authenticator(
+    caosdb_authentication_authenticator *authenticator),
+  {
+    if (authenticator->_deletable) {
+      delete static_cast<caosdb::authentication::Authenticator *>(
+        authenticator->wrapped_authenticator);
+    }
+    authenticator->_deletable = false;
+    return 0;
+  })
 
 ERROR_RETURN_CODE(
   GENERIC_ERROR,
@@ -249,6 +258,7 @@ ERROR_RETURN_CODE(
       out->wrapped_connection_configuration =
         new caosdb::configuration::TlsConnectionConfiguration(host_str, port);
     }
+    out->_deletable = true;
     return 0;
   })
 
@@ -260,6 +270,7 @@ ERROR_RETURN_CODE(
   {
     out->wrapped_connection_configuration =
       new caosdb::configuration::InsecureConnectionConfiguration(host, port);
+    out->_deletable = true;
     return 0;
   })
 
@@ -268,8 +279,11 @@ ERROR_RETURN_CODE(
   int caosdb_connection_delete_connection_configuration(
     caosdb_connection_connection_configuration *configuration),
   {
-    delete static_cast<caosdb::configuration::ConnectionConfiguration *>(
-      configuration->wrapped_connection_configuration);
+    if (configuration->_deletable) {
+      delete static_cast<caosdb::configuration::ConnectionConfiguration *>(
+        configuration->wrapped_connection_configuration);
+    }
+    configuration->_deletable = false;
     return 0;
   })
 
@@ -283,6 +297,7 @@ ERROR_RETURN_CODE(
       static_cast<caosdb::configuration::ConnectionConfiguration *>(
         configuration->wrapped_connection_configuration);
     out->wrapped_connection = new caosdb::connection::Connection(*config);
+    out->_deletable = true;
     return 0;
   })
 
@@ -290,8 +305,11 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_connection_delete_connection(
                     caosdb_connection_connection *connection),
                   {
-                    delete static_cast<caosdb::connection::Connection *>(
-                      connection->wrapped_connection);
+                    if (connection->_deletable) {
+                      delete static_cast<caosdb::connection::Connection *>(
+                        connection->wrapped_connection);
+                    }
+                    connection->_deletable = false;
                     return 0;
                   })
 
@@ -347,6 +365,9 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                       caosdb::connection::ConnectionManager::GetConnection(
                         std::string(name))
                         .get();
+		    // managed by the connection manager now, so not
+		    // to be deleted manually
+		    out->_deletable = false;
                     return 0;
                   })
 
@@ -363,6 +384,7 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                         connection->wrapped_connection);
                     out->wrapped_transaction =
                       wrapped_connection->CreateTransaction().release();
+                    out->_deletable = true;
                     return 0;
                   })
 
@@ -370,8 +392,10 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_transaction_delete_transaction(
                     caosdb_transaction_transaction *transaction),
                   {
-                    delete static_cast<caosdb::transaction::Transaction *>(
-                      transaction->wrapped_transaction);
+                    if (transaction->_deletable) {
+                      delete static_cast<caosdb::transaction::Transaction *>(
+                        transaction->wrapped_transaction);
+                    }
                     return 0;
                   })
 
@@ -476,13 +500,17 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
 ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_entity_create_entity(caosdb_entity_entity *out), {
                     out->wrapped_entity = new caosdb::entity::Entity();
+                    out->_deletable = true;
                     return 0;
                   })
 
 ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_entity_delete_entity(caosdb_entity_entity *out), {
-                    delete static_cast<caosdb::entity::Entity *>(
-                      out->wrapped_entity);
+                    if (out->_deletable) {
+                      delete static_cast<caosdb::entity::Entity *>(
+                        out->wrapped_entity);
+                    }
+                    out->_deletable = false;
                     return 0;
                   })
 
@@ -490,26 +518,34 @@ ERROR_RETURN_CODE(
   GENERIC_ERROR, int caosdb_entity_create_property(caosdb_entity_property *out),
   {
     out->wrapped_property = new caosdb::entity::Property();
+    out->_deletable = true;
     return 0;
   })
 
 ERROR_RETURN_CODE(
   GENERIC_ERROR, int caosdb_entity_delete_property(caosdb_entity_property *out),
   {
-    delete static_cast<caosdb::entity::Property *>(out->wrapped_property);
+    if (out->_deletable) {
+      delete static_cast<caosdb::entity::Property *>(out->wrapped_property);
+    }
+    out->_deletable = false;
     return 0;
   })
 
 ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_entity_create_parent(caosdb_entity_parent *out), {
                     out->wrapped_parent = new caosdb::entity::Parent();
+                    out->_deletable = true;
                     return 0;
                   })
 
 ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_entity_delete_parent(caosdb_entity_parent *out), {
-                    delete static_cast<caosdb::entity::Parent *>(
-                      out->wrapped_parent);
+                    if (out->_deletable) {
+                      delete static_cast<caosdb::entity::Parent *>(
+                        out->wrapped_parent);
+                    }
+                    out->_deletable = false;
                     return 0;
                   })
 
@@ -543,6 +579,7 @@ ERROR_RETURN_CODE(
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
     out->wrapped_message = wrapped_entity->GetErrors().mutable_at(index);
+    out->_deletable = false;
     return 0;
   })
 
@@ -565,6 +602,7 @@ ERROR_RETURN_CODE(
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
     out->wrapped_message = wrapped_entity->GetWarnings().mutable_at(index);
+    out->_deletable = false;
     return 0;
   })
 
@@ -587,6 +625,7 @@ ERROR_RETURN_CODE(
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
     out->wrapped_message = wrapped_entity->GetInfos().mutable_at(index);
+    out->_deletable = false;
     return 0;
   })
 
@@ -609,6 +648,7 @@ ERROR_RETURN_CODE(
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
     out->wrapped_property = wrapped_entity->GetProperties().mutable_at(index);
+    out->_deletable = false;
     return 0;
   })
 
@@ -631,6 +671,7 @@ ERROR_RETURN_CODE(
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
     out->wrapped_parent = wrapped_entity->GetParents().mutable_at(index);
+    out->_deletable = false;
     return 0;
   })
 

test/test_ccaosdb.cpp

@@ -324,13 +324,13 @@ TEST_F(test_ccaosdb, test_entity_with_parent_and_property) {
   return_code = caosdb_entity_delete_entity(&entity);
   EXPECT_EQ(return_code, 0);
 
-  // TODO(fspreck) This fails, because the output_p{arent,roperty} is owned
-  // by the entity (which is already deleted). Also, this shows that it is
-  // dangerous to call the deletion of output_p{arent,roperty} before the
-  // entity is being deleted, because it screws up the RepeatedPtrFieldWrapper.
+  // This tests the `_deletable` flag. The wrapped cpp objects of
+  // `output_parent` and `output_property` are owned by the entity, so
+  // they have been deleted together with the entity. With a wrong
+  // `_deletable` flag, the following would cause segfaults.
   //
-  // return_code = caosdb_entity_delete_parent(&output_parent);
-  // EXPECT_EQ(return_code, 0);
-  // return_code = caosdb_entity_delete_property(&output_property);
-  // EXPECT_EQ(return_code, 0);
+  return_code = caosdb_entity_delete_parent(&output_parent);
+  EXPECT_EQ(return_code, 0);
+  return_code = caosdb_entity_delete_property(&output_property);
+  EXPECT_EQ(return_code, 0);
 }