Skip to content

Commit 6260e42

Browse files
committed
update
1 parent a5b6817 commit 6260e42

File tree

2 files changed

+27
-34
lines changed

2 files changed

+27
-34
lines changed

ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,8 @@ class HttpClient : public opentelemetry::ext::http::client::HttpClient
356356
std::unique_ptr<std::thread> background_thread_;
357357
std::chrono::milliseconds scheduled_delay_milliseconds_;
358358

359-
std::condition_variable background_thread_waiter_cv_;
360-
std::mutex background_thread_waiter_lock_;
361359
std::chrono::milliseconds background_thread_wait_for_;
360+
std::atomic<bool> is_shutdown;
362361

363362
nostd::shared_ptr<HttpCurlGlobalInitializer> curl_global_initializer_;
364363
};

ext/src/http/client/curl/http_client_curl.cc

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,15 @@ HttpClient::HttpClient()
191191
scheduled_delay_milliseconds_{std::chrono::milliseconds(256)},
192192
background_thread_wait_for_{std::chrono::minutes{1}},
193193
curl_global_initializer_(HttpCurlGlobalInitializer::GetInstance())
194-
{}
194+
{
195+
is_shutdown.store(false);
196+
}
195197

196198
HttpClient::~HttpClient()
197199
{
200+
is_shutdown.store(true, std::memory_order_release);
198201
while (true)
199202
{
200-
background_thread_wait_for_ = std::chrono::milliseconds{0};
201203
std::unique_ptr<std::thread> background_thread;
202204
{
203205
std::lock_guard<std::mutex> lock_guard{background_thread_m_};
@@ -213,7 +215,6 @@ HttpClient::~HttpClient()
213215
}
214216
if (background_thread->joinable())
215217
{
216-
background_thread_waiter_cv_.notify_all();
217218
background_thread->join();
218219
}
219220
}
@@ -239,7 +240,6 @@ std::shared_ptr<opentelemetry::ext::http::client::Session> HttpClient::CreateSes
239240
std::lock_guard<std::mutex> lock_guard{sessions_m_};
240241
sessions_.insert({session_id, session});
241242

242-
background_thread_waiter_cv_.notify_all();
243243
// FIXME: Session may leak if it do not call SendRequest
244244
return session;
245245
}
@@ -350,18 +350,28 @@ void HttpClient::MaybeSpawnBackgroundThread()
350350
background_thread_.reset(new std::thread(
351351
[](HttpClient *self) {
352352
int still_running = 1;
353+
std::chrono::system_clock::time_point last_free_job_timepoint =
354+
std::chrono::system_clock::now();
353355
while (true)
354356
{
355357
CURLMsg *msg;
356358
int queued;
357359
CURLMcode mc = curl_multi_perform(self->multi_handle_, &still_running);
360+
361+
std::chrono::system_clock::time_point now = std::chrono::system_clock::now();
362+
363+
auto wait_for = self->background_thread_wait_for_;
364+
if (self->is_shutdown.load(std::memory_order_acquire))
365+
{
366+
wait_for = std::chrono::milliseconds{0};
367+
}
358368
// According to https://curl.se/libcurl/c/curl_multi_perform.html, when mc is not OK, we
359369
// can not curl_multi_perform it again
360370
if (mc != CURLM_OK)
361371
{
362372
self->resetMultiHandle();
363373
}
364-
else if (still_running)
374+
else if (still_running || now - last_free_job_timepoint < wait_for)
365375
{
366376
// curl_multi_poll is added from libcurl 7.66.0, before 7.68.0, we can only wait util
367377
// timeout to do the rest jobs
@@ -422,22 +432,11 @@ void HttpClient::MaybeSpawnBackgroundThread()
422432

423433
if (still_running > 0)
424434
{
435+
last_free_job_timepoint = now;
425436
continue;
426437
}
427438

428-
// If there is no pending jobs, Exit flush thread if there is not data to flush more
429-
// than one minute.
430-
if (self->background_thread_wait_for_ != std::chrono::milliseconds{0})
431-
{
432-
std::unique_lock<std::mutex> lk{self->background_thread_waiter_lock_};
433-
if (self->background_thread_waiter_cv_.wait_for(
434-
lk, self->background_thread_wait_for_) != std::cv_status::timeout)
435-
{
436-
continue;
437-
}
438-
}
439-
440-
if (still_running == 0)
439+
if (still_running == 0 && now - last_free_job_timepoint > wait_for)
441440
{
442441
std::lock_guard<std::mutex> lock_guard{self->background_thread_m_};
443442
// Double check, make sure no more pending sessions after locking background thread
@@ -461,18 +460,16 @@ void HttpClient::MaybeSpawnBackgroundThread()
461460
still_running = 1;
462461
}
463462

464-
if (still_running > 0)
465-
{
466-
continue;
467-
}
468-
469463
// If there is no pending jobs, we can stop the background thread.
470-
if (self->background_thread_)
464+
if (still_running == 0)
471465
{
472-
self->background_thread_->detach();
473-
self->background_thread_.reset();
466+
if (self->background_thread_)
467+
{
468+
self->background_thread_->detach();
469+
self->background_thread_.reset();
470+
}
471+
break;
474472
}
475-
break;
476473
}
477474
}
478475
},
@@ -533,6 +530,7 @@ void HttpClient::SetBackgroundWaitFor(std::chrono::milliseconds ms)
533530

534531
void HttpClient::WaitBackgroundThreadExit()
535532
{
533+
is_shutdown.store(true, std::memory_order_release);
536534
std::unique_ptr<std::thread> background_thread;
537535
{
538536
std::lock_guard<std::mutex> lock_guard{background_thread_m_};
@@ -541,12 +539,9 @@ void HttpClient::WaitBackgroundThreadExit()
541539

542540
if (background_thread && background_thread->joinable())
543541
{
544-
auto wait_for = background_thread_wait_for_;
545-
background_thread_wait_for_ = std::chrono::milliseconds{0};
546-
background_thread_waiter_cv_.notify_all();
547542
background_thread->join();
548-
background_thread_wait_for_ = wait_for;
549543
}
544+
is_shutdown.store(false, std::memory_order_release);
550545
}
551546

552547
void HttpClient::wakeupBackgroundThread()
@@ -560,7 +555,6 @@ void HttpClient::wakeupBackgroundThread()
560555
curl_multi_wakeup(multi_handle_);
561556
}
562557
#endif
563-
background_thread_waiter_cv_.notify_all();
564558
}
565559

566560
bool HttpClient::doAddSessions()

0 commit comments

Comments
 (0)