Skip to content

Commit c1c6783

Browse files
[CDRIVER-5604] Change updates to SRV rescan interval to be atomic (#1640)
Because the topology's rescan interval can be updated in parallel with other threads reading its value, the value needs to be atomic to prevent data races. This does not necessarily fix race conditions, but does avoid the UB of data races.
1 parent de2467e commit c1c6783

File tree

3 files changed

+29
-22
lines changed

3 files changed

+29
-22
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ static BSON_THREAD_FUN (srv_polling_run, topology_void)
5353
/* Unlock and sleep until next scan is due, or until shutdown signalled.
5454
*/
5555
now_ms = bson_get_monotonic_time () / 1000;
56-
scan_due_ms = topology->srv_polling_last_scan_ms + topology->srv_polling_rescan_interval_ms;
56+
scan_due_ms = topology->srv_polling_last_scan_ms + _mongoc_topology_get_srv_polling_rescan_interval_ms (topology);
5757
sleep_duration_ms = scan_due_ms - now_ms;
5858

5959
if (sleep_duration_ms > 0) {

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

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ typedef struct _mongoc_topology_t {
128128
int64_t min_heartbeat_frequency_msec;
129129

130130
/* Minimum of SRV record TTLs, but no lower than 60 seconds.
131-
* May be zero for non-SRV/non-MongoS topology. */
132-
int64_t srv_polling_rescan_interval_ms;
131+
* May be zero for non-SRV/non-MongoS topology.
132+
* DO NOT access directly: use the accessor methods to get/set the value.
133+
*/
134+
int64_t _atomic_srv_polling_rescan_interval_ms;
133135
int64_t srv_polling_last_scan_ms;
134136
/* For multi-threaded, srv polling occurs in a separate thread. */
135137
bson_thread_t srv_polling_thread;
@@ -440,12 +442,23 @@ mongoc_topology_should_rescan_srv (mongoc_topology_t *topology);
440442
void
441443
_mongoc_topology_set_rr_resolver (mongoc_topology_t *topology, _mongoc_rr_resolver_fn rr_resolver);
442444

443-
/* _mongoc_topology_set_srv_polling_rescan_interval_ms is called by tests to
444-
* shorten the rescan interval.
445-
* Callers should call this before monitoring starts.
445+
/**
446+
* @brief Thread-safe update the SRV polling rescan interval on the given topology
446447
*/
447-
void
448-
_mongoc_topology_set_srv_polling_rescan_interval_ms (mongoc_topology_t *topology, int64_t val);
448+
static inline void
449+
_mongoc_topology_set_srv_polling_rescan_interval_ms (mongoc_topology_t *topology, int64_t val)
450+
{
451+
bson_atomic_int64_exchange (&topology->_atomic_srv_polling_rescan_interval_ms, val, bson_memory_order_seq_cst);
452+
}
453+
454+
/**
455+
* @brief Thread-safe get the SRV polling interval
456+
*/
457+
static inline int64_t
458+
_mongoc_topology_get_srv_polling_rescan_interval_ms (mongoc_topology_t const *topology)
459+
{
460+
return bson_atomic_int64_fetch (&topology->_atomic_srv_polling_rescan_interval_ms, bson_memory_order_seq_cst);
461+
}
449462

450463
/**
451464
* @brief Return the latest connection generation for the server_id and/or

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

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded)
480480
* lookup fails, SRV polling will still start when background monitoring
481481
* starts. */
482482
topology->srv_polling_last_scan_ms = bson_get_monotonic_time () / 1000;
483-
topology->srv_polling_rescan_interval_ms = MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS;
483+
_mongoc_topology_set_srv_polling_rescan_interval_ms (topology, MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS);
484484

485485
/* a mongodb+srv URI. try SRV lookup, if no error then also try TXT */
486486
prefixed_hostname = bson_strdup_printf ("_%s._tcp.%s", mongoc_uri_get_srv_service_name (uri), srv_hostname);
@@ -518,8 +518,8 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded)
518518

519519
topology->srv_polling_last_scan_ms = bson_get_monotonic_time () / 1000;
520520
/* TODO (CDRIVER-4047) use BSON_MIN */
521-
topology->srv_polling_rescan_interval_ms =
522-
BSON_MAX (rr_data.min_ttl * 1000, MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS);
521+
int64_t new_iv = BSON_MAX (rr_data.min_ttl * 1000, MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS);
522+
_mongoc_topology_set_srv_polling_rescan_interval_ms (topology, new_iv);
523523

524524
topology->valid = true;
525525
srv_fail:
@@ -816,7 +816,7 @@ mongoc_topology_rescan_srv (mongoc_topology_t *topology)
816816
BSON_ASSERT (mongoc_topology_should_rescan_srv (topology));
817817

818818
srv_hostname = mongoc_uri_get_srv_hostname (topology->uri);
819-
scan_time_ms = topology->srv_polling_last_scan_ms + topology->srv_polling_rescan_interval_ms;
819+
scan_time_ms = topology->srv_polling_last_scan_ms + _mongoc_topology_get_srv_polling_rescan_interval_ms (topology);
820820
if (bson_get_monotonic_time () / 1000 < scan_time_ms) {
821821
/* Query SRV no more frequently than srv_polling_rescan_interval_ms. */
822822
return;
@@ -839,14 +839,14 @@ mongoc_topology_rescan_srv (mongoc_topology_t *topology)
839839
topology->srv_polling_last_scan_ms = bson_get_monotonic_time () / 1000;
840840
if (!ret) {
841841
/* Failed querying, soldier on and try again next time. */
842-
topology->srv_polling_rescan_interval_ms = td.ptr->heartbeat_msec;
842+
_mongoc_topology_set_srv_polling_rescan_interval_ms (topology, td.ptr->heartbeat_msec);
843843
MONGOC_ERROR ("SRV polling error: %s", topology->scanner->error.message);
844844
GOTO (done);
845845
}
846846

847847
/* TODO (CDRIVER-4047) use BSON_MIN */
848-
topology->srv_polling_rescan_interval_ms =
849-
BSON_MAX (rr_data.min_ttl * 1000, MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS);
848+
const int64_t new_iv = BSON_MAX (rr_data.min_ttl * 1000, MONGOC_TOPOLOGY_MIN_RESCAN_SRV_INTERVAL_MS);
849+
_mongoc_topology_set_srv_polling_rescan_interval_ms (topology, new_iv);
850850

851851
tdmod = mc_tpld_modify_begin (topology);
852852
if (!mongoc_topology_apply_scanned_srv_hosts (
@@ -861,7 +861,7 @@ mongoc_topology_rescan_srv (mongoc_topology_t *topology)
861861
* to heartbeatFrequencyMS until at least one verified SRV record is
862862
* obtained."
863863
*/
864-
topology->srv_polling_rescan_interval_ms = td.ptr->heartbeat_msec;
864+
_mongoc_topology_set_srv_polling_rescan_interval_ms (topology, td.ptr->heartbeat_msec);
865865
}
866866
mc_tpld_modify_commit (tdmod);
867867

@@ -1889,12 +1889,6 @@ _mongoc_topology_set_rr_resolver (mongoc_topology_t *topology, _mongoc_rr_resolv
18891889
topology->rr_resolver = rr_resolver;
18901890
}
18911891

1892-
void
1893-
_mongoc_topology_set_srv_polling_rescan_interval_ms (mongoc_topology_t *topology, int64_t val)
1894-
{
1895-
topology->srv_polling_rescan_interval_ms = val;
1896-
}
1897-
18981892
uint32_t
18991893
_mongoc_topology_get_connection_pool_generation (const mongoc_topology_description_t *td,
19001894
uint32_t server_id,

0 commit comments

Comments
 (0)