CDRIVER-3909 topology description copy/destroy methods (#785)

* Rename mongoc_topology_description_destroy to cleanup

* Use consistent language in server description copy/destroy docs

* CDRIVER-3909 topology description copy/destroy methods

Co-authored-by: Kevin Albertson <[email protected]>

Author: jmikola

src/libmongoc/doc/mongoc_server_description_destroy.rst

@@ -19,4 +19,4 @@ Parameters
 Description
 -----------
 
-Clean up all memory associated with the server description. Does nothing if ``description`` is NULL.
+Frees all resources associated with the server description. Does nothing if ``description`` is NULL.

src/libmongoc/doc/mongoc_server_description_new_copy.rst

@@ -20,10 +20,9 @@ Parameters
 Description
 -----------
 
-This function copies the given server description and returns a new server description object.  The caller is responsible for destroying the new copy.
+Performs a deep copy of ``description``.
 
 Returns
 -------
 
-A copy of the original server description.
-
+Returns a newly allocated copy of ``description`` that should be freed with :symbol:`mongoc_server_description_destroy()` when no longer in use. Returns NULL if ``description`` is NULL.

src/libmongoc/doc/mongoc_topology_description_destroy.rst

@@ -0,0 +1,22 @@
+:man_page: mongoc_topology_description_destroy
+
+mongoc_topology_description_destroy()
+=====================================
+
+Synopsis
+--------
+
+.. code-block:: c
+
+  void
+  mongoc_topology_description_destroy (mongoc_topology_description_t *description);
+
+Parameters
+----------
+
+* ``description``: A :symbol:`mongoc_topology_description_t`.
+
+Description
+-----------
+
+Frees all resources associated with the topology description. Does nothing if ``description`` is NULL.

src/libmongoc/doc/mongoc_topology_description_new_copy.rst

@@ -0,0 +1,28 @@
+:man_page: mongoc_topology_description_new_copy
+
+mongoc_topology_description_new_copy()
+======================================
+
+Synopsis
+--------
+
+.. code-block:: c
+
+  mongoc_topology_description_t *
+  mongoc_topology_description_new_copy (
+     const mongoc_topology_description_t *description);
+
+Parameters
+----------
+
+* ``description``: A :symbol:`mongoc_topology_description_t`.
+
+Description
+-----------
+
+Performs a deep copy of ``description``.
+
+Returns
+-------
+
+Returns a newly allocated copy of ``description`` that should be freed with :symbol:`mongoc_topology_description_destroy()` when no longer in use. Returns NULL if ``description`` is NULL.

src/libmongoc/doc/mongoc_topology_description_t.rst

@@ -15,7 +15,7 @@ Synopsis
 ``mongoc_topology_description_t`` is an opaque type representing the driver's knowledge of the MongoDB server or servers it is connected to.
 Its API conforms to the `SDAM Monitoring Specification <https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring-monitoring.rst>`_.
 
-Applications receive a temporary reference to a ``mongoc_topology_description_t`` as a parameter to an SDAM Monitoring callback. See :doc:`Introduction to Application Performance Monitoring <application-performance-monitoring>`.
+Applications receive a temporary reference to a ``mongoc_topology_description_t`` as a parameter to an SDAM Monitoring callback that must not be destroyed. See :doc:`Introduction to Application Performance Monitoring <application-performance-monitoring>`.
 
 .. only:: html
 
@@ -26,8 +26,10 @@ Applications receive a temporary reference to a ``mongoc_topology_description_t`
     :titlesonly:
     :maxdepth: 1
 
+    mongoc_topology_description_destroy
     mongoc_topology_description_get_servers
     mongoc_topology_description_has_readable_server
     mongoc_topology_description_has_writable_server
+    mongoc_topology_description_new_copy
     mongoc_topology_description_type
 

src/libmongoc/src/mongoc/mongoc-topology-description-apm.c

@@ -132,7 +132,7 @@ _mongoc_topology_description_monitor_opening (mongoc_topology_description_t *td)
       prev_sd = mongoc_server_description_new_copy (sd);
       BSON_ASSERT (prev_sd);
       if (td->apm_callbacks.topology_changed) {
-         mongoc_topology_description_destroy (prev_td);
+         mongoc_topology_description_cleanup (prev_td);
          _mongoc_topology_description_copy_to (td, prev_td);
       }
       sd->type = MONGOC_SERVER_LOAD_BALANCER;
@@ -144,7 +144,7 @@ _mongoc_topology_description_monitor_opening (mongoc_topology_description_t *td)
    }
 
    if (prev_td) {
-      mongoc_topology_description_destroy (prev_td);
+      mongoc_topology_description_cleanup (prev_td);
       bson_free (prev_td);
    }
 }

src/libmongoc/src/mongoc/mongoc-topology-description-private.h

@@ -73,7 +73,7 @@ _mongoc_topology_description_copy_to (const mongoc_topology_description_t *src,
                                       mongoc_topology_description_t *dst);
 
 void
-mongoc_topology_description_destroy (
+mongoc_topology_description_cleanup (
    mongoc_topology_description_t *description);
 
 void

src/libmongoc/src/mongoc/mongoc-topology-description.c

@@ -104,7 +104,7 @@ mongoc_topology_description_init (mongoc_topology_description_t *description,
  *
  *       Deep-copy @src to an uninitialized topology description @dst.
  *       @dst must not already point to any allocated resources. Clean
- *       up with mongoc_topology_description_destroy.
+ *       up with mongoc_topology_description_cleanup.
  *
  *       WARNING: @dst's rand_seed is not initialized.
  *
@@ -163,12 +163,46 @@ _mongoc_topology_description_copy_to (const mongoc_topology_description_t *src,
    EXIT;
 }
 
+/*
+ *-------------------------------------------------------------------------
+ *
+ * mongoc_topology_description_new_copy --
+ *
+ *       Allocates a new topology description and deep-copies @description to it
+ *       using _mongoc_topology_description_copy_to.
+ *
+ * Returns:
+ *       A copy of a topology description that you must destroy with
+ *       mongoc_topology_description_destroy, or NULL if @description is NULL.
+ *
+ * Side effects:
+ *       None.
+ *
+ *-------------------------------------------------------------------------
+ */
+mongoc_topology_description_t *
+mongoc_topology_description_new_copy (
+   const mongoc_topology_description_t *description)
+{
+   mongoc_topology_description_t *copy;
+
+   if (!description) {
+      return NULL;
+   }
+
+   copy = (mongoc_topology_description_t *) bson_malloc0 (sizeof (*copy));
+
+   _mongoc_topology_description_copy_to (description, copy);
+
+   return copy;
+}
+
 /*
  *--------------------------------------------------------------------------
  *
- * mongoc_topology_description_destroy --
+ * mongoc_topology_description_cleanup --
  *
- *       Destroy allocated resources within @description
+ *       Destroy allocated resources within @description but don't free it.
  *
  * Returns:
  *       None.
@@ -179,7 +213,7 @@ _mongoc_topology_description_copy_to (const mongoc_topology_description_t *src,
  *--------------------------------------------------------------------------
  */
 void
-mongoc_topology_description_destroy (mongoc_topology_description_t *description)
+mongoc_topology_description_cleanup (mongoc_topology_description_t *description)
 {
    ENTRY;
 
@@ -198,6 +232,37 @@ mongoc_topology_description_destroy (mongoc_topology_description_t *description)
    EXIT;
 }
 
+/*
+ *--------------------------------------------------------------------------
+ *
+ * mongoc_topology_description_destroy --
+ *
+ *       Destroy allocated resources within @description and free
+ *       @description.
+ *
+ * Returns:
+ *       None.
+ *
+ * Side effects:
+ *       None.
+ *
+ *--------------------------------------------------------------------------
+ */
+void
+mongoc_topology_description_destroy (mongoc_topology_description_t *description)
+{
+   ENTRY;
+
+   if (!description) {
+      EXIT;
+   }
+
+   mongoc_topology_description_cleanup (description);
+   bson_free (description);
+
+   EXIT;
+}
+
 /* find the primary, then stop iterating */
 static bool
 _mongoc_topology_description_has_primary_cb (void *item, void *ctx /* OUT */)
@@ -1984,7 +2049,7 @@ mongoc_topology_description_handle_hello (
              &sd->topology_version, &incoming_topology_version) == 1) {
          TRACE ("%s", "topology version is strictly less. Skipping.");
          if (prev_td) {
-            mongoc_topology_description_destroy (prev_td);
+            mongoc_topology_description_cleanup (prev_td);
             bson_free (prev_td);
          }
          return;
@@ -2070,7 +2135,7 @@ mongoc_topology_description_handle_hello (
    }
 
    if (prev_td) {
-      mongoc_topology_description_destroy (prev_td);
+      mongoc_topology_description_cleanup (prev_td);
       bson_free (prev_td);
    }
 

src/libmongoc/src/mongoc/mongoc-topology-description.h

@@ -29,14 +29,25 @@ BSON_BEGIN_DECLS
 
 typedef struct _mongoc_topology_description_t mongoc_topology_description_t;
 
+MONGOC_EXPORT (void)
+mongoc_topology_description_destroy (
+   mongoc_topology_description_t *description);
+
+MONGOC_EXPORT (mongoc_topology_description_t *)
+mongoc_topology_description_new_copy (
+   const mongoc_topology_description_t *description);
+
 MONGOC_EXPORT (bool)
 mongoc_topology_description_has_readable_server (
    mongoc_topology_description_t *td, const mongoc_read_prefs_t *prefs);
+
 MONGOC_EXPORT (bool)
 mongoc_topology_description_has_writable_server (
    mongoc_topology_description_t *td);
+
 MONGOC_EXPORT (const char *)
 mongoc_topology_description_type (const mongoc_topology_description_t *td);
+
 MONGOC_EXPORT (mongoc_server_description_t **)
 mongoc_topology_description_get_servers (
    const mongoc_topology_description_t *td, size_t *n);

src/libmongoc/src/mongoc/mongoc-topology.c

@@ -567,7 +567,7 @@ mongoc_topology_destroy (mongoc_topology_t *topology)
    _mongoc_topology_description_monitor_closed (&topology->description);
 
    mongoc_uri_destroy (topology->uri);
-   mongoc_topology_description_destroy (&topology->description);
+   mongoc_topology_description_cleanup (&topology->description);
    mongoc_topology_scanner_destroy (topology->scanner);
 
    /* If we are single-threaded, the client will try to call

src/libmongoc/tests/json-test.c

@@ -543,7 +543,7 @@ test_server_selection_logic_cb (bson_t *test)
 
 DONE:
    mongoc_read_prefs_destroy (read_prefs);
-   mongoc_topology_description_destroy (&topology);
+   mongoc_topology_description_cleanup (&topology);
    _mongoc_array_destroy (&selected_servers);
 }
 

src/libmongoc/tests/test-mongoc-dns.c

@@ -579,7 +579,7 @@ test_srv_polling_mocked (void *unused)
    _mongoc_host_list_destroy_all (hosts);
    ASSERT_CAPTURED_LOG ("topology", MONGOC_LOG_LEVEL_ERROR, "Invalid host");
 
-   mongoc_topology_description_destroy (&td);
+   mongoc_topology_description_cleanup (&td);
    mongoc_uri_destroy (uri);
 }
 

src/libmongoc/tests/test-mongoc-topology-description.c

@@ -208,6 +208,52 @@ test_topology_version_equal (void)
    mongoc_uri_destroy (uri);
 }
 
+static void
+test_topology_description_new_copy (void)
+{
+   mongoc_uri_t *uri;
+   mongoc_topology_t *topology;
+   mongoc_topology_description_t *td, *td_copy;
+   mongoc_server_description_t *sd_a;
+   mongoc_server_description_t *sd_c;
+   mongoc_server_description_t **sds;
+   size_t n;
+
+   uri = mongoc_uri_new ("mongodb://a,b,c");
+   topology = mongoc_topology_new (uri, true /* single-threaded */);
+   td = &topology->description;
+
+   td_copy = mongoc_topology_description_new_copy (td);
+
+   /* servers "a" and "c" are mongos, but "b" remains unknown */
+   sd_a = _sd_for_host (td, "a");
+   mongoc_topology_description_handle_hello (
+      td, sd_a->id, tmp_bson ("{'ok': 1, 'msg': 'isdbgrid'}"), 100, NULL);
+
+   sd_c = _sd_for_host (td, "c");
+   mongoc_topology_description_handle_hello (
+      td, sd_c->id, tmp_bson ("{'ok': 1, 'msg': 'isdbgrid'}"), 100, NULL);
+
+   /* td was copied before original was updated */
+   sds = mongoc_topology_description_get_servers (td_copy, &n);
+   ASSERT_CMPSIZE_T ((size_t) 0, ==, n);
+
+   mongoc_server_descriptions_destroy_all (sds, n);
+   mongoc_topology_description_destroy (td_copy);
+
+   td_copy = mongoc_topology_description_new_copy (td);
+
+   mongoc_topology_destroy (topology);
+   mongoc_uri_destroy (uri);
+
+   /* td was copied after original was updated, but before it was destroyed */
+   sds = mongoc_topology_description_get_servers (td_copy, &n);
+   ASSERT_CMPSIZE_T ((size_t) 2, ==, n);
+
+   mongoc_server_descriptions_destroy_all (sds, n);
+   mongoc_topology_description_destroy (td_copy);
+}
+
 void
 test_topology_description_install (TestSuite *suite)
 {
@@ -221,4 +267,7 @@ test_topology_description_install (TestSuite *suite)
    TestSuite_Add (suite,
                   "/TopologyDescription/topology_version_equal",
                   test_topology_version_equal);
+   TestSuite_Add (suite,
+                  "/TopologyDescription/new_copy",
+                  test_topology_description_new_copy);
 }