Skip to content

Commit 06e319b

Browse files
jpnurmiclaude
andcommitted
bring in on_timeout from the http-retry branch
When shutdown times out, the on_timeout callback cancels the in-flight HTTP request (via curl progress callback abort or WinHTTP handle closure), unblocking the worker thread so it can finish remaining tasks like cache cleanup. Fixes test_cache_max_items failing on Windows where WinHTTP blocks for seconds on unreachable hosts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 48c69e8 commit 06e319b

5 files changed

Lines changed: 75 additions & 19 deletions

File tree

src/sentry_sync.c

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,8 @@ shutdown_task(void *task_data, void *UNUSED(state))
425425
}
426426

427427
int
428-
sentry__bgworker_shutdown(sentry_bgworker_t *bgw, uint64_t timeout)
428+
sentry__bgworker_shutdown_cb(sentry_bgworker_t *bgw, uint64_t timeout,
429+
void (*on_timeout)(void *), void *on_timeout_data)
429430
{
430431
if (!sentry__atomic_fetch(&bgw->running)) {
431432
SENTRY_WARN("trying to shut down non-running thread");
@@ -441,12 +442,23 @@ sentry__bgworker_shutdown(sentry_bgworker_t *bgw, uint64_t timeout)
441442
while (true) {
442443
uint64_t now = sentry__monotonic_time();
443444
if (now > started && now - started > timeout) {
444-
sentry__atomic_store(&bgw->running, 0);
445-
sentry__thread_detach(bgw->thread_id);
446-
sentry__mutex_unlock(&bgw->task_lock);
447-
SENTRY_WARN(
448-
"background thread failed to shut down cleanly within timeout");
449-
return 1;
445+
if (on_timeout) {
446+
// fire on_timeout to cancel the ongoing task, and give the
447+
// worker an extra loop cycle up to 250ms to handle the
448+
// cancellation
449+
sentry__mutex_unlock(&bgw->task_lock);
450+
on_timeout(on_timeout_data);
451+
on_timeout = NULL;
452+
sentry__mutex_lock(&bgw->task_lock);
453+
// fall through to !running check below
454+
} else {
455+
sentry__atomic_store(&bgw->running, 0);
456+
sentry__thread_detach(bgw->thread_id);
457+
sentry__mutex_unlock(&bgw->task_lock);
458+
SENTRY_WARN("background thread failed to shut down cleanly "
459+
"within timeout");
460+
return 1;
461+
}
450462
}
451463

452464
if (!sentry__atomic_fetch(&bgw->running)) {

src/sentry_sync.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,20 @@ int sentry__bgworker_flush(sentry_bgworker_t *bgw, uint64_t timeout);
469469
/**
470470
* This will try to shut down the background worker thread, with a `timeout`.
471471
* Returns 0 on success.
472+
*
473+
* The `_cb` variant accepts an `on_timeout` callback that is invoked when
474+
* the timeout expires, just before detaching the thread. This gives the
475+
* caller a chance to unblock the worker (e.g. by closing transport handles)
476+
* so it can finish in-flight work.
472477
*/
473-
int sentry__bgworker_shutdown(sentry_bgworker_t *bgw, uint64_t timeout);
478+
int sentry__bgworker_shutdown_cb(sentry_bgworker_t *bgw, uint64_t timeout,
479+
void (*on_timeout)(void *), void *on_timeout_data);
480+
481+
static inline int
482+
sentry__bgworker_shutdown(sentry_bgworker_t *bgw, uint64_t timeout)
483+
{
484+
return sentry__bgworker_shutdown_cb(bgw, timeout, NULL, NULL);
485+
}
474486

475487
/**
476488
* This will set a preferable thread name for background worker.

src/transports/sentry_http_transport.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,15 @@ http_send_task(void *_envelope, void *_state)
252252
}
253253
}
254254

255+
static void
256+
http_transport_shutdown_timeout(void *_state)
257+
{
258+
http_transport_state_t *state = _state;
259+
if (state->shutdown_client) {
260+
state->shutdown_client(state->client);
261+
}
262+
}
263+
255264
static int
256265
http_transport_start(const sentry_options_t *options, void *transport_state)
257266
{
@@ -288,10 +297,8 @@ http_transport_shutdown(uint64_t timeout, void *transport_state)
288297
sentry_bgworker_t *bgworker = transport_state;
289298
http_transport_state_t *state = sentry__bgworker_get_state(bgworker);
290299

291-
int rv = sentry__bgworker_shutdown(bgworker, timeout);
292-
if (rv != 0 && state->shutdown_client) {
293-
state->shutdown_client(state->client);
294-
}
300+
int rv = sentry__bgworker_shutdown_cb(
301+
bgworker, timeout, http_transport_shutdown_timeout, state);
295302
return rv;
296303
}
297304

src/transports/sentry_http_transport_curl.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "sentry_http_transport.h"
55
#include "sentry_options.h"
66
#include "sentry_string.h"
7+
#include "sentry_sync.h"
78
#include "sentry_transport.h"
89
#include "sentry_utils.h"
910

@@ -20,6 +21,7 @@ typedef struct {
2021
char *proxy;
2122
char *ca_certs;
2223
bool debug;
24+
long shutdown;
2325
#ifdef SENTRY_PLATFORM_NX
2426
void *nx_state;
2527
#endif
@@ -117,6 +119,22 @@ curl_client_start(void *_client, const sentry_options_t *options)
117119
return 0;
118120
}
119121

122+
static void
123+
curl_client_shutdown(void *_client)
124+
{
125+
curl_client_t *client = _client;
126+
sentry__atomic_store(&client->shutdown, 1);
127+
}
128+
129+
static int
130+
progress_callback(void *clientp, curl_off_t UNUSED(dltotal),
131+
curl_off_t UNUSED(dlnow), curl_off_t UNUSED(ultotal),
132+
curl_off_t UNUSED(ulnow))
133+
{
134+
curl_client_t *client = clientp;
135+
return sentry__atomic_fetch(&client->shutdown) ? 1 : 0;
136+
}
137+
120138
static size_t
121139
swallow_data(
122140
char *UNUSED(ptr), size_t size, size_t nmemb, void *UNUSED(userdata))
@@ -189,6 +207,10 @@ curl_send_task(void *_client, sentry_prepared_http_request_t *req,
189207
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, req->body);
190208
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, (long)req->body_len);
191209
curl_easy_setopt(curl, CURLOPT_USERAGENT, SENTRY_SDK_USER_AGENT);
210+
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, 15000L);
211+
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
212+
curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, progress_callback);
213+
curl_easy_setopt(curl, CURLOPT_XFERINFODATA, client);
192214

193215
char error_buf[CURL_ERROR_SIZE];
194216
error_buf[0] = 0;
@@ -253,5 +275,6 @@ sentry__transport_new_default(void)
253275
}
254276
sentry__http_transport_set_free_client(transport, curl_client_free);
255277
sentry__http_transport_set_start_client(transport, curl_client_start);
278+
sentry__http_transport_set_shutdown_client(transport, curl_client_shutdown);
256279
return transport;
257280
}

src/transports/sentry_http_transport_winhttp.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ winhttp_client_start(void *_client, const sentry_options_t *opts)
134134
return 1;
135135
}
136136

137+
// 15s resolve/connect, 30s send/receive (WinHTTP defaults)
138+
WinHttpSetTimeouts(client->session, 15000, 15000, 30000, 30000);
139+
137140
return 0;
138141
}
139142

@@ -154,9 +157,9 @@ winhttp_client_shutdown(void *_client)
154157
WinHttpCloseHandle(client->session);
155158
client->session = NULL;
156159
}
157-
if (client->request) {
158-
WinHttpCloseHandle(client->request);
159-
client->request = NULL;
160+
HINTERNET request = InterlockedExchangePointer(&client->request, NULL);
161+
if (request) {
162+
WinHttpCloseHandle(request);
160163
}
161164
}
162165

@@ -302,10 +305,9 @@ winhttp_send_task(void *_client, sentry_prepared_http_request_t *req,
302305
uint64_t now = sentry__monotonic_time();
303306
SENTRY_DEBUGF("request handled in %llums", now - started);
304307

305-
exit:
306-
if (client->request) {
307-
HINTERNET request = client->request;
308-
client->request = NULL;
308+
exit:;
309+
HINTERNET request = InterlockedExchangePointer(&client->request, NULL);
310+
if (request) {
309311
WinHttpCloseHandle(request);
310312
}
311313
sentry_free(url);

0 commit comments

Comments
 (0)