Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix EPOLLET event loss and coroutine frame leak at shutdown
This PR fixes two long-standing bugs in the event loop that surface
under load with bursty I/O and consumers that don't drain the kernel
buffer fast enough.
1. EPOLLET event loss when no awaiter is registered
Symptom
Long-running sockets eventually hang.
pg_stat_activityshows thebackend in
ClientReadwhiless -tnpishows several MB sitting inthe socket's receive queue. The client never reads, the server never
gets NACKed, and the connection idles forever.
Root cause
EPollerregisters fds withEPOLLIN | EPOLLOUT | EPOLLET. Withedge-triggered semantics, the kernel only re-fires
EPOLLINwhen thebuffer transitions from empty to non-empty. If
EPOLLINarrives at amoment when no coroutine is parked in
sock->first(for example,between two awaits where the consumer is briefly busy), the wakeup is
silently dropped. The next
co_await wait_readable()parks itself andwaits forever, because the kernel will not re-fire until the buffer
drains, which it never does.
Fix
Two new bits per socket header —
READ_PENDING_MASKandWRITE_PENDING_MASK— packed into the existingstateword(bits 57-58, alongside the unused bits above
TIMEOUT_EPOCH_MASK).EPoller: ifEPOLLIN/EPOLLOUTarrives andsock->first/secondis null, set the corresponding pending bit instead of discarding
the event.
AwaiterRead/AwaiterWrite/AwaiterAccept::await_ready:consume the pending bit and proceed without suspending if it was set.
AwaiterRead/AwaiterWrite/AwaiterAccept::await_suspend:after registering into the slot and clearing busy, re-check pending.
If a wakeup landed in the small window between
await_readyand thestore into
first/second, self-resume by enqueueing into theready queue. Closes the race without requiring a full lock.
Files:
include/uvent/utils/sync/RefCountedSession.hinclude/uvent/net/SocketMetadata.hsrc/poll/EPoller.cppsrc/net/AwaiterOperations.cpp2. Coroutine frame leak at shutdown (~350 MB on a small workload)
Symptom
ASAN under
-fsanitize=addressreports leaks ofAwaitable<...>framesallocated by
pump_input,wait_readable, and similar — over a millionallocations on a workload that only opens 16 connections. Even small
test programs leak hundreds of MB by exit time.
Root cause
AwaitableFrame::final_suspendreturnssuspend_always. After acoroutine returns, its frame pushes itself into thread-local
q_c,the destruction queue.
q_cis drained inside the main worker loop(
while (!token.stop_requested())). WhenUvent::stop()is called,the loop exits before processing the last batch of pending destructions.
The frames sitting in
q_cat exit time are leaked.Fix
Thread::threadFunction, after the mainwhileexits and before theQSBR detach, drains
q_c(andq_shunderUVENT_ENABLE_REUSEADDR)to empty in a tight loop. Each destroyed handle is overwritten with
nullptrintmp_coroutines_to keep the buffer consistent.Drains live in
threadFunctionrather than~Thread()becausethread-local
q_cis only reachable from inside the worker thread.Files:
src/system/Thread.cppTesting
-fsanitize=address,undefinedand run a workload thatopens 16 connections, executes ~1M coroutine resumes across 8 large
exports, and exits via
Uvent::stop().intermittent heap-use-after-free at shutdown.
Thread.cpp:66(nullEPollervptr in early init under REUSEADDR) and
Thread.cpp:80(nullcoroutine_handlewrite through the bulk-dequeue buffer) areunrelated to this PR and left for a separate change.