Skip to content

Commit 5a7e671

Browse files
committed
[CHORE] Say each invariant once in the comments
The comments carried the reasoning that found the bug as well as the rule the code follows. The rule is what a reader needs; the rest belongs in the pull request. Each block now states its constraint and stops, and the two member comments in the installed headers follow the one line trailing form the file already uses next to them. No code changes. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent e625eef commit 5a7e671

2 files changed

Lines changed: 6 additions & 11 deletions

File tree

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -817,10 +817,8 @@ bool HttpClient::doRemoveSessions()
817817
continue;
818818
}
819819

820-
// Take it out of the multi handle first. libcurl does not allow an easy handle to be
821-
// cleaned up while a multi handle still owns it, and the header list has to stay alive
822-
// until the handle is no longer used for a transfer. A handle that was never added
823-
// reports CURLM_OK here, so this does not need to know which is which.
820+
// Remove before cleanup: libcurl forbids freeing an easy handle, or the header list it
821+
// points at, while the multi handle still owns it. A handle never added reports CURLM_OK.
824822
const CURLMcode rc = curl_multi_remove_handle(multi_handle_, resource.easy_handle);
825823
if (CURLM_OK != rc)
826824
{

‎ext/src/http/client/curl/http_operation_curl.cc‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -542,10 +542,8 @@ void HttpOperation::Cleanup()
542542
// Only cleanup async once even in recursive calls
543543
if (async_data_)
544544
{
545-
// Move the easy handle to the owner untouched. It may still be attached to the multi handle
546-
// and running, and the thread inside curl_multi_perform owns it until the IO thread takes it
547-
// out, so writing to it here would be a write to a live transfer. It is pointless as well:
548-
// the next thing that happens to the handle is curl_easy_cleanup.
545+
// Hand the easy handle over untouched. It may still be attached to the multi handle, and the
546+
// thread inside curl_multi_perform owns it until the IO thread removes it.
549547
Session *session = async_data_->session.exchange(nullptr, std::memory_order_acq_rel);
550548
if (session != nullptr)
551549
{
@@ -1525,9 +1523,8 @@ void HttpOperation::PerformCurlMessage(CURLcode code)
15251523
{
15261524
if (is_cleaned_.load(std::memory_order_acquire))
15271525
{
1528-
// Already torn down, and the handle is queued for removal. The multi handle can still hold a
1529-
// message buffered from before that, and acting on it would dispatch a second round of
1530-
// terminal events and can put a handle that is waiting to be freed back on the retry queue.
1526+
// Already torn down and queued for removal. A message buffered before that would dispatch a
1527+
// second round of terminal events and requeue a handle waiting to be freed.
15311528
return;
15321529
}
15331530

0 commit comments

Comments
 (0)