CDRIVER-1895 clean up topology locking in callback

Author: ajdavis

src/mongoc/mongoc-topology-scanner-private.h

@@ -35,6 +35,10 @@
 
 BSON_BEGIN_DECLS
 
+typedef void (*mongoc_topology_scanner_setup_err_cb_t)(uint32_t            id,
+                                                       void               *data,
+                                                       const bson_error_t *error /* IN */);
+
 typedef void (*mongoc_topology_scanner_cb_t)(uint32_t            id,
                                              const bson_t       *bson,
                                              int64_t             rtt,
@@ -74,14 +78,15 @@ typedef struct mongoc_topology_scanner
    bool                            handshake_ok_to_send;
    const char                     *appname;
 
-   mongoc_topology_scanner_cb_t    cb;
-   void                           *cb_data;
-   bool                            in_progress;
-   const mongoc_uri_t             *uri;
-   mongoc_async_cmd_setup_t        setup;
-   mongoc_stream_initiator_t       initiator;
-   void                           *initiator_context;
-   bson_error_t                    error;
+   mongoc_topology_scanner_setup_err_cb_t  setup_err_cb;
+   mongoc_topology_scanner_cb_t            cb;
+   void                                   *cb_data;
+   bool                                    in_progress;
+   const mongoc_uri_t                     *uri;
+   mongoc_async_cmd_setup_t                setup;
+   mongoc_stream_initiator_t               initiator;
+   void                                   *initiator_context;
+   bson_error_t                            error;
 
 #ifdef MONGOC_ENABLE_SSL
    mongoc_ssl_opt_t *ssl_opts;
@@ -92,9 +97,10 @@ typedef struct mongoc_topology_scanner
 } mongoc_topology_scanner_t;
 
 mongoc_topology_scanner_t *
-mongoc_topology_scanner_new (const mongoc_uri_t          *uri,
-                             mongoc_topology_scanner_cb_t cb,
-                             void                        *data);
+mongoc_topology_scanner_new (const mongoc_uri_t                     *uri,
+                             mongoc_topology_scanner_setup_err_cb_t  setup_err_cb,
+                             mongoc_topology_scanner_cb_t            cb,
+                             void                                   *data);
 
 void
 mongoc_topology_scanner_destroy (mongoc_topology_scanner_t *ts);

src/mongoc/mongoc-topology-scanner.c

@@ -127,9 +127,10 @@ _begin_ismaster_cmd (mongoc_topology_scanner_t      *ts,
 
 
 mongoc_topology_scanner_t *
-mongoc_topology_scanner_new (const mongoc_uri_t          *uri,
-                             mongoc_topology_scanner_cb_t cb,
-                             void                        *data)
+mongoc_topology_scanner_new (const mongoc_uri_t                     *uri,
+                             mongoc_topology_scanner_setup_err_cb_t  setup_err_cb,
+                             mongoc_topology_scanner_cb_t            cb,
+                             void                                   *data)
 {
    mongoc_topology_scanner_t *ts = (mongoc_topology_scanner_t *)bson_malloc0 (sizeof (*ts));
 
@@ -139,6 +140,7 @@ mongoc_topology_scanner_new (const mongoc_uri_t          *uri,
    _add_ismaster (&ts->ismaster_cmd);
    bson_init (&ts->ismaster_cmd_with_handshake);
 
+   ts->setup_err_cb = setup_err_cb;
    ts->cb = cb;
    ts->cb_data = data;
    ts->uri = uri;
@@ -396,6 +398,7 @@ mongoc_topology_scanner_ismaster_handler (mongoc_async_cmd_result_t async_status
    ts->cb (node->id, ismaster_response, rtt_msec, ts->cb_data, error);
 }
 
+
 /*
  *--------------------------------------------------------------------------
  *
@@ -598,8 +601,7 @@ mongoc_topology_scanner_node_setup (mongoc_topology_scanner_node_t *node,
       _mongoc_topology_scanner_monitor_heartbeat_failed (node->ts, &node->host,
                                                          error);
 
-      /* Pass a rtt of -1 if we couldn't initialize a stream in node_setup */
-      node->ts->cb (node->id, NULL, -1, node->ts->cb_data, error);
+      node->ts->setup_err_cb (node->id, node->ts->cb_data, error);
       return false;
    }
 

src/mongoc/mongoc-topology.c

@@ -99,6 +99,36 @@ _mongoc_topology_update_no_lock (uint32_t            id,
 }
 
 
+/*
+ *-------------------------------------------------------------------------
+ *
+ * _mongoc_topology_scanner_setup_err_cb --
+ *
+ *       Callback method to handle errors during topology scanner node
+ *       setup, typically DNS or SSL errors.
+ *
+ *-------------------------------------------------------------------------
+ */
+
+void
+_mongoc_topology_scanner_setup_err_cb (uint32_t id,
+                                       void *data,
+                                       const bson_error_t *error /* IN */)
+{
+   mongoc_topology_t *topology;
+
+   BSON_ASSERT (data);
+
+   topology = (mongoc_topology_t *) data;
+
+   mongoc_topology_description_handle_ismaster (&topology->description,
+                                                id,
+                                                NULL /* ismaster reply */,
+                                                -1 /* rtt_msec */,
+                                                error);
+}
+
+
 /*
  *-------------------------------------------------------------------------
  *
@@ -125,22 +155,12 @@ _mongoc_topology_scanner_cb (uint32_t            id,
 
    topology = (mongoc_topology_t *)data;
 
-   if (rtt_msec >= 0) {
-      /* If the scanner failed to create a socket for this server, that means
-       * we're in scanner_start, which means we're under the mutex.  So don't
-       * take the mutex for rtt < 0 */
-
-      mongoc_mutex_lock (&topology->mutex);
-   }
-
+   mongoc_mutex_lock (&topology->mutex);
    _mongoc_topology_update_no_lock (id, ismaster_response, rtt_msec, topology,
                                     error);
 
    mongoc_cond_broadcast (&topology->cond_client);
-
-   if (rtt_msec >= 0) {
-      mongoc_mutex_unlock (&topology->mutex);
-   }
+   mongoc_mutex_unlock (&topology->mutex);
 }
 
 /*
@@ -206,8 +226,10 @@ mongoc_topology_new (const mongoc_uri_t *uri,
    topology->uri = mongoc_uri_copy (uri);
    topology->scanner_state = MONGOC_TOPOLOGY_SCANNER_OFF;
    topology->scanner = mongoc_topology_scanner_new (topology->uri,
+                                                    _mongoc_topology_scanner_setup_err_cb,
                                                     _mongoc_topology_scanner_cb,
                                                     topology);
+
    topology->single_threaded = single_threaded;
    if (single_threaded) {
       /* Server Selection Spec:

tests/test-mongoc-topology-scanner.c

@@ -57,7 +57,7 @@ _test_topology_scanner(bool with_ssl)
 #endif
 
    topology_scanner = mongoc_topology_scanner_new (
-         NULL, &test_topology_scanner_helper, &finished);
+         NULL, NULL, &test_topology_scanner_helper, &finished);
 
 #ifdef MONGOC_ENABLE_SSL
    if (with_ssl) {

tests/test-mongoc-topology.c

@@ -929,6 +929,61 @@ test_rtt (void *ctx)
 }
 
 
+/* mongoc_topology_scanner_add_and_scan is called within the topology mutex, it
+ * adds a discovered node and calls getaddrinfo on its host immediately - test
+ * that this doesn't cause a recursive acquire on the topology mutex */
+static void
+test_add_and_scan_failure (void)
+{
+   mock_server_t *server;
+   mongoc_uri_t *uri;
+   mongoc_client_pool_t *pool;
+   mongoc_client_t *client;
+   future_t *future;
+   request_t *request;
+   bson_error_t error;
+   mongoc_server_description_t *sd;
+
+   server = mock_server_new ();
+   mock_server_run (server);
+   /* client will discover "fake" host and fail to connect */
+   mock_server_auto_ismaster (server,
+                              "{'ok': 1,"
+                              " 'ismaster': true,"
+                              " 'setName': 'rs',"
+                              " 'hosts': ['%s', 'fake:1']}",
+                              mock_server_get_host_and_port (server));
+
+   uri = mongoc_uri_copy (mock_server_get_uri (server));
+   mongoc_uri_set_option_as_utf8 (uri, "replicaSet", "rs");
+   pool = mongoc_client_pool_new (uri);
+   client = mongoc_client_pool_pop (pool);
+   future = future_client_command_simple (client, "db",
+                                          tmp_bson ("{'ping': 1}"),
+                                          NULL, NULL, &error);
+
+   request = mock_server_receives_command (server, "db", MONGOC_QUERY_NONE,
+                                           "{'ping': 1}");
+   mock_server_replies_ok_and_destroys (request);
+   ASSERT_OR_PRINT (future_get_bool (future), error);
+
+   sd = mongoc_topology_server_by_id (client->topology, 1, NULL);
+   ASSERT (sd);
+   ASSERT_CMPSTR (mongoc_server_description_type (sd), "RSPrimary");
+   mongoc_server_description_destroy (sd);
+
+   sd = mongoc_topology_server_by_id (client->topology, 2, NULL);
+   ASSERT (sd);
+   ASSERT_CMPSTR (mongoc_server_description_type (sd), "Unknown");
+   mongoc_server_description_destroy (sd);
+
+   future_destroy (future);
+   mongoc_client_pool_push (pool, client);
+   mongoc_client_pool_destroy (pool);
+   mongoc_uri_destroy (uri);
+   mock_server_destroy (server);
+}
+
 void
 test_topology_install (TestSuite *suite)
 {
@@ -967,4 +1022,6 @@ test_topology_install (TestSuite *suite)
                   test_server_removed_during_handshake_pooled);
    TestSuite_AddFull (suite, "/Topology/rtt", test_rtt, NULL, NULL,
                       test_framework_skip_if_slow);
+   TestSuite_AddLive (suite, "/Topology/add_and_scan_failure",
+                      test_add_and_scan_failure);
 }