Skip to content

Commit a675fdd

Browse files
committed
Fix kqueue false connect success from stale EVFILT_WRITE
The kqueue backend registers sockets for EVFILT_WRITE at open() time. A freshly created socket is writable, so kqueue fires a stale event before connect() completes. If the reactor processes this before the kernel delivers the connect result (e.g. RST for ECONNREFUSED), getsockopt(SO_ERROR) returns 0 and the connect falsely reports success. Fix by adding a getpeername() check in connect perform_io() to verify the connection is actually established when SO_ERROR is 0, returning EAGAIN to re-park the op if not. Add EAGAIN handling for connect ops in descriptor_state::operator()() to match the existing read/write pattern.
1 parent 504d294 commit a675fdd

File tree

2 files changed

+50
-3
lines changed

2 files changed

+50
-3
lines changed

include/boost/corosio/native/detail/kqueue/kqueue_op.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,21 @@ struct kqueue_connect_op final : kqueue_op
282282
socklen_t len = sizeof(err);
283283
if (::getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &len) < 0)
284284
err = errno;
285+
// Guard against stale EVFILT_WRITE events from socket creation.
286+
// kqueue registers EVFILT_WRITE at open() time; a freshly created
287+
// socket is "writable" so the filter fires immediately. If the
288+
// reactor processes this event after connect() returns EINPROGRESS
289+
// but before the kernel delivers the connect result (e.g. RST for
290+
// ECONNREFUSED), SO_ERROR is still 0. Use getpeername() to verify
291+
// the connection is actually established.
292+
if (err == 0)
293+
{
294+
sockaddr_storage peer{};
295+
socklen_t peer_len = sizeof(peer);
296+
if (::getpeername(
297+
fd, reinterpret_cast<sockaddr*>(&peer), &peer_len) < 0)
298+
err = EAGAIN;
299+
}
285300
complete(err, 0);
286301
}
287302

include/boost/corosio/native/detail/kqueue/kqueue_scheduler.hpp

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -604,8 +604,16 @@ descriptor_state::operator()()
604604
cn->complete(err, 0);
605605
else
606606
cn->perform_io();
607-
local_ops.push(cn);
608-
cn = nullptr;
607+
608+
if (cn->errn == EAGAIN || cn->errn == EWOULDBLOCK)
609+
{
610+
cn->errn = 0;
611+
}
612+
else
613+
{
614+
local_ops.push(cn);
615+
cn = nullptr;
616+
}
609617
}
610618

611619
if (wr)
@@ -630,7 +638,7 @@ descriptor_state::operator()()
630638
// have set read_ready/write_ready while we held the op (no read_op
631639
// was registered, so it cached the edge event). Check the flags
632640
// under the same lock as re-registration so no edge is lost.
633-
while (rd || wr)
641+
while (rd || wr || cn)
634642
{
635643
bool retry = false;
636644
{
@@ -661,6 +669,19 @@ descriptor_state::operator()()
661669
wr = nullptr;
662670
}
663671
}
672+
if (cn)
673+
{
674+
if (write_ready)
675+
{
676+
write_ready = false;
677+
retry = true;
678+
}
679+
else
680+
{
681+
connect_op = cn;
682+
cn = nullptr;
683+
}
684+
}
664685
}
665686

666687
if (!retry)
@@ -688,6 +709,17 @@ descriptor_state::operator()()
688709
wr = nullptr;
689710
}
690711
}
712+
if (cn)
713+
{
714+
cn->perform_io();
715+
if (cn->errn == EAGAIN || cn->errn == EWOULDBLOCK)
716+
cn->errn = 0;
717+
else
718+
{
719+
local_ops.push(cn);
720+
cn = nullptr;
721+
}
722+
}
691723
}
692724

693725
// Execute first handler inline — the scheduler's work_cleanup

0 commit comments

Comments
 (0)