(Maybe) Rare multithreading + socket reuse race condition causing crash

[radkesvat]

Hi, first of all, thank you for this excellent project.

I recently encountered a rare multithreading issue in my project using libhv as its event loop, and I would like to share the scenario to confirm if it is a bug or a misunderstanding on my side.

---

Context:

In my project, I removed the mutex for hio_t (though I believe this is unrelated to the issue).

The scenario is:

• We have two threads:

  • Main thread: creates a TCP server on port 443.
  • Worker thread: sockets accepted on the main thread are distributed to either the main thread or the worker thread using hio_detach / hio_attach.

• When a thread gets an accepted socket, it also initiates a connection to an external TCP address (e.g., google.com:443).

---

Example state:

At a given moment we have these sockets:

• s1: accepted socket on thread 0 (main).
• s2: connection to google on thread 0 (main).
• s3: accepted from thread 0 but moved to thread 1 (worker).
• s4: connection to google on thread 1 (worker).

note that i didnt mention the socket that is accepting connections on main thread

---

The problematic event order:

Assume epoll on the main thread returns three events in this order:

1. Data ready to read on s2.
2. Data ready to read on the server accept FD (a new connection is ready to be accepted).
3. Data ready to read on s1.

During processing of event 1, reading the data causes the logic to close its paired socket (s1).

Then during event 2, accept may return a new socket FD that reuses the same FD number as s1 (since it was just closed).

If the main thread assigns this newly accepted socket to thread 1 and then proceeds to event 3, it attempts to read from the hio_t associated with the old s1 (which is now invalid for the main thread’s event loop). The loop->ios.ptr[fd] is NULL, but the pointer used during pending event processing is still valid and is now incorrectly transferred to thread 1, since hio_t is being reused

---

My current workaround:

Before actually closing the socket FD in hio_close, I check if there are pending events for the hio_t. If so, I queue a message to call close on the next iteration rather than closing immediately, ensuring no pending events are associated with the hio_t during closure.

---

If this analysis is correct and the issue is indeed present, I would be happy to prepare a pull request with my current fix, or adjust it if you believe there is a cleaner solution.

Thank you for your time and for maintaining libhv!

[ithewei]

In a loop, the fd is closed, the same fd is accepted again, and there are events related to the fd in event pending. This is indeed a very coincidental case. The io should be removed（or marked as not to trigger event callback） from the event pending queue after hio_close (that called hio_del).
Try applying the following patch to see if it solves the problem.

diff --git a/event/hevent.h b/event/hevent.h
index be03897..b8d287e 100644
--- a/event/hevent.h
+++ b/event/hevent.h
@@ -269,6 +269,14 @@ void hio_memmove_readbuf(hio_t* io);
         }\
     } while(0)

+#define EVENT_UNPENDING(ev) \
+    do {\
+        if (ev->pending) {\
+            ev->pending = 0;\
+            ev->loop->npendings--;\
+        }\
+    } while(0)
+
 #define EVENT_ADD(loop, ev, cb) \
     do {\
         ev->loop = loop;\
diff --git a/event/hloop.c b/event/hloop.c
index 9f31331..498316e 100644
--- a/event/hloop.c
+++ b/event/hloop.c
@@ -875,6 +875,7 @@ int hio_del(hio_t* io, int events) {
         io->loop->nios--;
         // NOTE: not EVENT_DEL, avoid free
         EVENT_INACTIVE(io);
+        EVENT_UNPENDING(io);
     }
     return 0;
 }

[radkesvat]

Sure, I’ll apply and test this, but I believe it might not work as expected because:

The pending mechanism uses a linked list-like structure and doesn’t strictly rely on npendings inside the loop.

During processing of pending event #2, the main thread accepts a new socket and calls hio_detach. At this point, two things can happen:

1. If the main thread is fast enough (e.g., has no other workload), it proceeds immediately to the next pending event and may skip it—since the pending flag has been cleared in the meantime.

2. Or, the worker thread quickly calls hio_attach, then hio_read, which causes epoll to mark the hio_t as pending again. When the main thread then tries to process the next pending event, it ends up reading from a socket that no longer belongs to it.

This could result in accessing a hio_t that was already reassigned to a different thread, leading to race condition and undefined behaviour

but i might be wrong so ill tell you if my app with your patch crashes or not, it usually needed 2-3 hours of work to finally crash

[radkesvat]

It’s been about an hour, and my application hasn’t crashed with your patch applied — that's a good sign.

I also ran a test server under stress, using both iperf and a DDoS simulation script that previously allowed me to consistently reproduce the bug for debugging purposes. With your patch, I’ve been unable to trigger the issue again, which strongly suggests the race condition has been addressed.

That said, it’s still a bit challenging for me to confirm with 100% certainty that the race is fully resolved at a logical level, but so far it has passed all of my practical tests.

Thanks again!

[ithewei]

Sure, I’ll apply and test this, but I believe it might not work as expected because:

The pending mechanism uses a linked list-like structure and doesn’t strictly rely on npendings inside the loop.

During processing of pending event #2, the main thread accepts a new socket and calls hio_detach. At this point, two things can happen:

1. If the main thread is fast enough (e.g., has no other workload), it proceeds immediately to the next pending event and may skip it—since the pending flag has been cleared in the meantime.
2. Or, the worker thread quickly calls hio_attach, then hio_read, which causes epoll to mark the hio_t as pending again. When the main thread then tries to process the next pending event, it ends up reading from a socket that no longer belongs to it.

This could result in accessing a hio_t that was already reassigned to a different thread, leading to race condition and undefined behaviour

but i might be wrong so ill tell you if my app with your patch crashes or not, it usually needed 2-3 hours of work to finally crash

This patch should prevent the second situation you mentioned.

diff --git a/event/hloop.c b/event/hloop.c
index 9f31331..2278ff8 100644
--- a/event/hloop.c
+++ b/event/hloop.c
@@ -117,7 +117,7 @@ static int hloop_process_pendings(hloop_t* loop) {
         cur = loop->pendings[i];
         while (cur) {
             next = cur->pending_next;
-            if (cur->pending) {
+            if (cur->pending && cur->loop == loop) {
                 if (cur->active && cur->cb) {
                     cur->cb(cur);
                     ++ncbs;

[radkesvat]

Yes, you’re absolutely right — and with this new patch, I can finally sleep well without worrying about crashes 😅

the older patch could also keep my app running untill now without any probelm

[ithewei]

If everything is fine, please submit a PR for this issue.