Skip to content

CDRIVER-5815 support disabling socket timeouts #2185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
connorsmacd wants to merge 28 commits into
base: master
Choose a base branch
from
+213 −96
Open

CDRIVER-5815 support disabling socket timeouts #2185

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2cf5adc
Add failing test
connorsmacd Dec 4, 2025
6ed9323
Fix socketTimeoutMS=0 ignored in URI
connorsmacd Dec 4, 2025
596727c
Treat 0 timeout as infinite
connorsmacd Dec 4, 2025
5432608
Fix meaning of 0 timeout for TLS
connorsmacd Dec 4, 2025
bb23cbd
Revert "Fix socketTimeoutMS=0 ignored in URI"
connorsmacd Dec 4, 2025
5678eec
Support socketTimeoutMS=inf
connorsmacd Dec 8, 2025
d4d1f45
Set SSL opts on test client
connorsmacd Dec 9, 2025
4fb27cb
Remove include
connorsmacd Dec 9, 2025
8c12d39
Merge branch 'master' into sockettimeoutms_0.CDRIVER-5815
connorsmacd Dec 9, 2025
d9811a7
Apply timeout changes to OpenSSL TLS
connorsmacd Dec 10, 2025
969fe26
Handle case where immediate timeout is desired
connorsmacd Dec 11, 2025
6e1a6db
Apply timeout channels to secure channel
connorsmacd Dec 11, 2025
52c1672
Fix non-blocking timeout handling in TLS
connorsmacd Dec 16, 2025
6d0b809
Fix handling of sub-millisecond timeouts
connorsmacd Dec 16, 2025
f8056fd
Refactor timeout handling to be less error-prone
connorsmacd Dec 17, 2025
2a458ef
Rename test case
connorsmacd Dec 17, 2025
2261cfd
Remove timing checks from test
connorsmacd Dec 17, 2025
93b32a8
Refer to future fix ticket number
connorsmacd Dec 17, 2025
dfac42d
Warn if socketTimeoutMS=0 is specified
connorsmacd Dec 17, 2025
3c59307
Add comment with ticket number
connorsmacd Dec 17, 2025
7f4a63c
Document socketTimeoutMS=inf
connorsmacd Dec 17, 2025
34f63a5
Document socketTimeoutMS=inf as a C Driver extension
connorsmacd Dec 18, 2025
6ff501c
Use more familiar time point comparison syntax
connorsmacd Dec 18, 2025
f65bb76
Move new constants into private header
connorsmacd Dec 18, 2025
fb8f40f
Fix inconsistent integer widths
connorsmacd Dec 18, 2025
1773b8a
Fix inconsistent east/west const
connorsmacd Dec 18, 2025
5314d22
Move MONGOC_DEFAULT_SOCKETTIMEOUTMS back to mongoc-client.h
connorsmacd Dec 18, 2025
4b7dce8
Fix fixed-width integer includes
connorsmacd Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/libmongoc/doc/mongoc_uri_t.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ MONGOC_URI_SRVMAXHOSTS srvmaxhosts 0
The meaning of a timeout of ``0`` or a negative value may vary depending on the operation being executed, even when specified by the same URI option.
To specify the documented default value for a \*timeoutMS option, use the `MONGOC_DEFAULT_*` constants defined in ``mongoc-client.h`` instead.

In the case of socketTimeoutMS, to disable the timeout completely (i.e., an infinite timeout), use ``socketTimeoutMS=inf``.

Authentication Options
----------------------

Expand Down
4 changes: 2 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-async-cmd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ mongoc_async_cmd_tls_setup(mongoc_stream_t *stream, int *events, void *ctx, mlib
// Try to do a non-blocking operation, if our backend allows it
const mlib_duration_rep_t remain_ms = //
use_non_blocking
// Pass 0 for the timeout to begin / continue a non-blocking handshake
? 0
// Pass sentinel value for the timeout to begin / continue a non-blocking handshake
? MONGOC_SOCKET_TIMEOUT_IMMEDIATE
// Otherwise, use the deadline
: mlib_milliseconds_count(mlib_timer_remaining(deadline));
if (mongoc_stream_tls_handshake(tls_stream, host, mlib_assert_narrow(int32_t, remain_ms), &retry_events, error)) {
Expand Down
13 changes: 0 additions & 13 deletions src/libmongoc/src/mongoc/mongoc-client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,6 @@ BSON_BEGIN_DECLS
#endif


#ifndef MONGOC_DEFAULT_SOCKETTIMEOUTMS
/*
* NOTE: The default socket timeout for connections is 5 minutes. This
* means that if your MongoDB server dies or becomes unavailable
* it will take 5 minutes to detect this.
*
* You can change this by providing sockettimeoutms= in your
* connection URI.
*/
#define MONGOC_DEFAULT_SOCKETTIMEOUTMS (1000L * 60L * 5L)
#endif


/**
* mongoc_client_t:
*
Expand Down
3 changes: 1 addition & 2 deletions src/libmongoc/src/mongoc/mongoc-cluster.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2502,8 +2502,7 @@ void
mongoc_cluster_reset_sockettimeoutms(mongoc_cluster_t *cluster)
{
BSON_ASSERT_PARAM(cluster);
cluster->sockettimeoutms =
mongoc_uri_get_option_as_int32(cluster->uri, MONGOC_URI_SOCKETTIMEOUTMS, MONGOC_DEFAULT_SOCKETTIMEOUTMS);
cluster->sockettimeoutms = mongoc_uri_get_socket_timeout_ms_option(cluster->uri);
}

static uint32_t
Expand Down
6 changes: 3 additions & 3 deletions src/libmongoc/src/mongoc/mongoc-stream-socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ struct _mongoc_stream_socket_t {
static BSON_INLINE int64_t
get_expiration(int32_t timeout_msec)
{
if (timeout_msec < 0) {
return -1;
} else if (timeout_msec == 0) {
if (timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE) {
return 0;
} else if (timeout_msec <= 0) {
Copy link
Collaborator

@kevinAlbs kevinAlbs Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this may change how 0 is interpreted when directly using the mongoc_stream_t API. The mongoc_stream_t API is (unfortunately) public and has known users (e.g. userver here). Example:

mongoc_socket_t *sock = mongoc_socket_new(AF_INET, SOCK_STREAM, 0);

// Build address struct for localhost:27017:
struct sockaddr_in server_addr = {0};
server_addr.sin_family = AF_INET;
server_addr.sin_port = htons(27017);
server_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);

ssize_t ok = mongoc_socket_connect(sock, (struct sockaddr *)&server_addr, sizeof(server_addr), -1);
ASSERT_CMPSSIZE_T(ok, ==, 0);

mongoc_stream_t *stream = mongoc_stream_socket_new(sock);
mongoc_iovec_t iov = {.iov_base = "foo", .iov_len = 3};
ok = mongoc_stream_writev(stream, &iov, 1, 0 /* timeout */); // Want to be non-blocking.
ASSERT_CMPSSIZE_T(ok, ==, 3);

mongoc_stream_destroy(stream);

Suggest reverting to preserve the meaning of 0 as non-blocking in the mongoc_stream_t API.

Copy link
Collaborator Author

@connorsmacd connorsmacd Dec 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is quite unfortunate. While I could use the POSIX convention just for the public mongoc_stream_t API, I do not think this is the best approach.

Take mongoc_stream_read with OpenSSL enabled for example:

  • When mongoc_stream_read is called, we convert timeout_msec from the POSIX convention to the spec convention, then pass it to stream->readv.
  • stream->readv is _mongoc_stream_tls_openssl_readv in this case, which eventually calls mongoc_stream_tls_openssl_bio_read.
  • In mongoc_stream_tls_openssl_bio_read, we call mongoc_stream_read again, so we convert the timeout_msec back to the POSIX convention.

If we continue to mix conventions like this, then we are just asking for errors. In my opinion, it would be best to use the POSIX convention as much as possible internally and use the spec convention only where needed for spec compliance. In the case of socketTimeoutMS, we can use the spec convention within the URI, but once we extract the timeout from the URI, we convert to the POSIX convention then stick with it. This would have the added benefit of being more intuitive and less error-prone when dealing with POSIX system calls that accept a timeout.

That said, I recognize that being unaligned with the spec in this regard is less than ideal. This also goes against the recommendation in CDRIVER-5815:

IMO, libmongoc should be changed to have 0 actually mean "unlimited"

Thoughts?

Copy link
Contributor

@eramongodb eramongodb Dec 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mongoc_socket_t and mongoc_stream_t API have been an unfortunate source of codebase and behavior inconsistencies that is difficult to address due to being public API. They are also primarily responsible for blocking further int32 vs. int64 correctness improvements (CDRIVER-4596 and CDRIVER-4599) which were were unable to be scoped for the v2 release, as noted in #1475:

Additionally, an attempt is made to document the unspecified/inconsistent behavior of non-positive timeout values for the URI option documentation and for functions that have a timeout parameter (i.e. mongoc_stream_*()). Scanning through how the timeout values are both represented and interpreted within libmongoc, it does not seem desirable to continue to support non-positive URI timeout values (even 0) despite historical (implicit) support.

Concerning "spec" vs. "POSIX" conventions, I am slightly in favor of limiting the POSIX convention to the mongoc-stream-socket.c component (where get_expiration() converts from spec to POSIX) + implementing specific workarounds for the stream/socket API, since in most contexts except for the socket/stream API, we use the spec convention. If we want to use the POSIX convention throughout the codebase, I expect there will be comparatively more changes needed for that than what this PR currently proposes (e.g. consider also how mlib_timer API treats < 0 as immediate timeout, whereas POSIX convention treats < 0 as infinite timeout).

Copy link
Collaborator Author

@connorsmacd connorsmacd Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the important context.

Upon reflection, I think I can avoid some of the mess I was previously concerned about by adding new private mongoc_stream_t functions that mirror the public API, but use the expected timeout convention. All internal uses of mongoc_stream_t can use the private API without changing timeout conventions. The public API can be reimplemented to delegate to the private API by converting timeouts to the correct convention.

return -1;
Copy link
Contributor

@eramongodb eramongodb Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return -1;
return -1; // Infinite.

Suggest drive-by documenting the meaning of -1 here.

} else {
return (bson_get_monotonic_time() + ((int64_t)timeout_msec * 1000L));
}
Expand Down
46 changes: 11 additions & 35 deletions src/libmongoc/src/mongoc/mongoc-stream-tls-openssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,13 @@ _mongoc_stream_tls_openssl_write(mongoc_stream_tls_t *tls, char *buf, size_t buf
{
mongoc_stream_tls_openssl_t *openssl = (mongoc_stream_tls_openssl_t *)tls->ctx;
ssize_t ret;
int64_t now;
int64_t expire = 0;
ENTRY;

BSON_ASSERT(tls);
BSON_ASSERT(buf);
BSON_ASSERT(buf_len);

if (tls->timeout_msec >= 0) {
expire = bson_get_monotonic_time() + (tls->timeout_msec * 1000);
}
const mlib_timer timer = _mongoc_stream_tls_timer_from_timeout_msec(tls->timeout_msec);

BSON_ASSERT(mlib_in_range(int, buf_len));
ret = BIO_write(openssl->bio, buf, (int)buf_len);
Expand All @@ -220,18 +216,10 @@ _mongoc_stream_tls_openssl_write(mongoc_stream_tls_t *tls, char *buf, size_t buf
return ret;
}

if (expire) {
now = bson_get_monotonic_time();
tls->timeout_msec = _mongoc_stream_tls_timer_to_timeout_msec(timer);

if ((expire - now) < 0) {
if (mlib_cmp(ret, <, buf_len)) {
mongoc_counter_streams_timeout_inc();
}

tls->timeout_msec = 0;
} else {
tls->timeout_msec = (expire - now) / 1000;
}
if (tls->timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE && mlib_cmp(ret, <, buf_len)) {
mongoc_counter_streams_timeout_inc();
}

RETURN(ret);
Expand Down Expand Up @@ -411,8 +399,6 @@ _mongoc_stream_tls_openssl_readv(
size_t i;
int read_ret;
size_t iov_pos = 0;
int64_t now;
int64_t expire = 0;
ENTRY;

BSON_ASSERT(tls);
Expand All @@ -421,9 +407,7 @@ _mongoc_stream_tls_openssl_readv(

tls->timeout_msec = timeout_msec;

if (timeout_msec >= 0) {
expire = bson_get_monotonic_time() + (timeout_msec * 1000UL);
}
const mlib_timer timer = _mongoc_stream_tls_timer_from_timeout_msec(timeout_msec);

for (i = 0; i < iovcnt; i++) {
iov_pos = 0;
Expand All @@ -443,24 +427,16 @@ _mongoc_stream_tls_openssl_readv(
return -1;
}

if (expire) {
now = bson_get_monotonic_time();
tls->timeout_msec = _mongoc_stream_tls_timer_to_timeout_msec(timer);

if ((expire - now) < 0) {
if (read_ret == 0) {
mongoc_counter_streams_timeout_inc();
if (tls->timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE && read_ret == 0) {
mongoc_counter_streams_timeout_inc();
#ifdef _WIN32
errno = WSAETIMEDOUT;
errno = WSAETIMEDOUT;
#else
errno = ETIMEDOUT;
errno = ETIMEDOUT;
#endif
RETURN(-1);
}

tls->timeout_msec = 0;
} else {
tls->timeout_msec = (expire - now) / 1000L;
}
RETURN(-1);
}

ret += read_ret;
Expand Down
8 changes: 8 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-stream-tls-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

#include <bson/bson.h>

#include <mlib/timer.h>

#ifdef MONGOC_ENABLE_SSL_OPENSSL
#include <openssl/ssl.h>
#endif
Expand Down Expand Up @@ -67,6 +69,12 @@ mongoc_stream_tls_new_with_secure_channel_cred(mongoc_stream_t *base_stream,
mongoc_shared_ptr secure_channel_cred_ptr) BSON_GNUC_WARN_UNUSED_RESULT;
#endif // MONGOC_ENABLE_SSL_SECURE_CHANNEL

mlib_timer
_mongoc_stream_tls_timer_from_timeout_msec(int64_t timeout_msec);

int64_t
_mongoc_stream_tls_timer_to_timeout_msec(mlib_timer timer);

BSON_END_DECLS

#endif /* MONGOC_STREAM_TLS_PRIVATE_H */
24 changes: 6 additions & 18 deletions src/libmongoc/src/mongoc/mongoc-stream-tls-secure-channel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,6 @@ _mongoc_stream_tls_secure_channel_readv(
ssize_t ret = 0;
size_t i;
size_t iov_pos = 0;
int64_t now;
int64_t expire = 0;

BSON_ASSERT(iov);
BSON_ASSERT(iovcnt);
Expand All @@ -695,9 +693,7 @@ _mongoc_stream_tls_secure_channel_readv(

tls->timeout_msec = timeout_msec;

if (timeout_msec >= 0) {
expire = bson_get_monotonic_time() + (timeout_msec * 1000UL);
}
const mlib_timer timer = _mongoc_stream_tls_timer_from_timeout_msec(timeout_msec);

for (i = 0; i < iovcnt; i++) {
iov_pos = 0;
Expand All @@ -716,20 +712,12 @@ _mongoc_stream_tls_secure_channel_readv(
RETURN(-1);
}

if (expire) {
now = bson_get_monotonic_time();

if ((expire - now) < 0) {
if (read_ret == 0) {
mongoc_counter_streams_timeout_inc();
errno = ETIMEDOUT;
RETURN(-1);
}
tls->timeout_msec = _mongoc_stream_tls_timer_to_timeout_msec(timer);

tls->timeout_msec = 0;
} else {
tls->timeout_msec = (expire - now) / 1000L;
}
if (tls->timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE && read_ret == 0) {
mongoc_counter_streams_timeout_inc();
errno = ETIMEDOUT;
RETURN(-1);
}

ret += read_ret;
Expand Down
44 changes: 10 additions & 34 deletions src/libmongoc/src/mongoc/mongoc-stream-tls-secure-transport.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,11 @@ _mongoc_stream_tls_secure_transport_write(mongoc_stream_t *stream, char *buf, si
mongoc_stream_tls_t *tls = (mongoc_stream_tls_t *)stream;
mongoc_stream_tls_secure_transport_t *secure_transport = (mongoc_stream_tls_secure_transport_t *)tls->ctx;
ssize_t write_ret;
int64_t now;
int64_t expire = 0;

ENTRY;
BSON_ASSERT(secure_transport);

if (tls->timeout_msec >= 0) {
expire = bson_get_monotonic_time() + (tls->timeout_msec * 1000UL);
}
const mlib_timer timer = _mongoc_stream_tls_timer_from_timeout_msec(tls->timeout_msec);

status = SSLWrite(secure_transport->ssl_ctx_ref, buf, buf_len, (size_t *)&write_ret);

Expand All @@ -139,18 +135,10 @@ _mongoc_stream_tls_secure_transport_write(mongoc_stream_t *stream, char *buf, si
RETURN(-1);
}

if (expire) {
now = bson_get_monotonic_time();

if ((expire - now) < 0) {
if (write_ret < (ssize_t)buf_len) {
mongoc_counter_streams_timeout_inc();
}
tls->timeout_msec = _mongoc_stream_tls_timer_to_timeout_msec(timer);

tls->timeout_msec = 0;
} else {
tls->timeout_msec = (expire - now) / 1000L;
}
if (tls->timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE && write_ret < (ssize_t)buf_len) {
mongoc_counter_streams_timeout_inc();
}

RETURN(write_ret);
Expand Down Expand Up @@ -287,8 +275,6 @@ _mongoc_stream_tls_secure_transport_readv(
size_t i;
size_t read_ret;
size_t iov_pos = 0;
int64_t now;
int64_t expire = 0;
size_t to_read;
size_t remaining_buf_size;
size_t remaining_to_read;
Expand All @@ -300,9 +286,7 @@ _mongoc_stream_tls_secure_transport_readv(

tls->timeout_msec = timeout_msec;

if (timeout_msec >= 0) {
expire = bson_get_monotonic_time() + (timeout_msec * 1000UL);
}
const mlib_timer timer = _mongoc_stream_tls_timer_from_timeout_msec(timeout_msec);

for (i = 0; i < iovcnt; i++) {
iov_pos = 0;
Expand All @@ -328,20 +312,12 @@ _mongoc_stream_tls_secure_transport_readv(
RETURN(-1);
}

if (expire) {
now = bson_get_monotonic_time();

if ((expire - now) < 0) {
if (read_ret == 0) {
mongoc_counter_streams_timeout_inc();
errno = ETIMEDOUT;
RETURN(-1);
}
tls->timeout_msec = _mongoc_stream_tls_timer_to_timeout_msec(timer);

tls->timeout_msec = 0;
} else {
tls->timeout_msec = (expire - now) / 1000L;
}
if (tls->timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE && read_ret == 0) {
mongoc_counter_streams_timeout_inc();
errno = ETIMEDOUT;
RETURN(-1);
}

ret += read_ret;
Expand Down
30 changes: 30 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-stream-tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,34 @@ mongoc_stream_tls_new_with_secure_channel_cred(mongoc_stream_t *base_stream,
}
#endif // MONGOC_ENABLE_SSL_SECURE_CHANNEL

mlib_timer
_mongoc_stream_tls_timer_from_timeout_msec(int64_t timeout_msec)
{
if (timeout_msec == MONGOC_SOCKET_TIMEOUT_IMMEDIATE) {
return mlib_expires_after(0, ms);
} else if (timeout_msec <= 0) {
return mlib_expires_never();
} else {
return mlib_expires_after(timeout_msec, ms);
}
}

int64_t
_mongoc_stream_tls_timer_to_timeout_msec(mlib_timer timer)
{
const mlib_timer never = mlib_expires_never();

if (mlib_time_cmp(timer.expires_at, never.expires_at) == mlib_equal) {
return MONGOC_SOCKET_TIMEOUT_INFINITE;
}

const int64_t remaining_msec = mlib_milliseconds_count(mlib_timer_remaining(timer));

if (remaining_msec <= 0) {
return MONGOC_SOCKET_TIMEOUT_IMMEDIATE;
} else {
return remaining_msec;
}
}

#endif
5 changes: 5 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@
#include <mongoc/mongoc-macros.h>
#include <mongoc/mongoc-socket.h>

#include <stdint.h>


BSON_BEGIN_DECLS


#define MONGOC_SOCKET_TIMEOUT_INFINITE 0
#define MONGOC_SOCKET_TIMEOUT_IMMEDIATE INT32_MIN

typedef struct _mongoc_stream_t mongoc_stream_t;

typedef struct _mongoc_stream_poll_t {
Expand Down
3 changes: 3 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-uri-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ _mongoc_uri_apply_query_string(mongoc_uri_t *uri, mstr_view options, bool from_d
int32_t
mongoc_uri_get_local_threshold_option(const mongoc_uri_t *uri);

int32_t
mongoc_uri_get_socket_timeout_ms_option(const mongoc_uri_t *uri);

bool
_mongoc_uri_requires_auth_negotiation(const mongoc_uri_t *uri);

Expand Down
Loading