Fix kqueue close: clear SO_LINGER instead of calling shutdown#150
Fix kqueue close: clear SO_LINGER instead of calling shutdown#150mvandeberg merged 1 commit intocppalliance:developfrom
Conversation
On macOS, close() with SO_LINGER {1,0} sends RST instead of FIN.
RST does not reliably trigger EV_EOF on the peer's kqueue, causing
pending reads to hang. The previous workaround called shutdown(SHUT_WR)
before close to force a FIN.
Replace this with the approach used by Asio's kqueue reactor: clear
SO_LINGER before close so the kernel sends FIN naturally. Also skip
the explicit EV_DELETE (deregister_descriptor) on both sockets and
acceptors, relying on kqueue's automatic cleanup when the fd is
closed — matching Asio's deregister_descriptor(closing=true) path.
Update socket throughput benchmark to match asio.
📝 WalkthroughWalkthroughThese changes modify the kqueue socket lifecycle to defer linger option cleanup and descriptor deregistration operations from the close_socket() method to shutdown() and destroy() paths, while introducing a private state flag to track user-set linger options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://150.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-17 21:42:00 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp (1)
996-1000:⚠️ Potential issue | 🟠 Major
close()path does not clear SO_LINGER — RST may still be sent on user-initiated close.The
destroy()andshutdown()paths both clear SO_LINGER before closing the fd, butclose()delegates directly toclose_socket()which just closes the fd without clearing linger. Sincefd_is set to -1 inclose_socket(), the subsequentdestroy()call can no longer reach the fd to clear linger (guard on line 923 fails).If a user sets
SO_LINGER{1,0}and then explicitly closes the socket, the kernel will still send RST instead of FIN on macOS — the exact scenario this PR aims to fix.Proposed fix — clear linger in close() before delegating
inline void kqueue_socket_service::close(io_object::handle& h) { - static_cast<kqueue_socket*>(h.get())->close_socket(); + auto* kq_impl = static_cast<kqueue_socket*>(h.get()); + if (kq_impl->user_set_linger_ && kq_impl->fd_ >= 0) + { + struct ::linger lg; + lg.l_onoff = 0; + lg.l_linger = 0; + ::setsockopt(kq_impl->fd_, SOL_SOCKET, SO_LINGER, &lg, sizeof(lg)); + } + kq_impl->close_socket(); }Alternatively, consider extracting the linger-clearing logic into a small helper to avoid the now three-way duplication (see next comment).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp` around lines 996 - 1000, The close path currently calls kqueue_socket::close_socket() directly and therefore doesn't clear SO_LINGER before the fd is closed (unlike destroy() and shutdown()), which allows an RST to be sent; modify kqueue_socket_service::close to clear the socket's SO_LINGER option on the underlying fd (the same linger-clearing logic used in destroy()/shutdown()) before delegating to kqueue_socket::close_socket(), or factor that linger-clearing code into a small helper (e.g., clear_socket_linger(fd) used by destroy(), shutdown(), and close()) so all three paths clear SO_LINGER prior to closing.
🧹 Nitpick comments (1)
include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp (1)
874-898: Consider extracting aclear_linger_if_neededhelper to reduce duplication.The same linger-clearing pattern (check
user_set_linger_+fd_ >= 0, thensetsockopt) is repeated inshutdown()anddestroy(), and should also appear inclose()(see previous comment). A small private helper onkqueue_socketwould consolidate the logic:Example helper
// In kqueue_socket: void clear_user_linger() noexcept { if (user_set_linger_ && fd_ >= 0) { struct ::linger lg; lg.l_onoff = 0; lg.l_linger = 0; ::setsockopt(fd_, SOL_SOCKET, SO_LINGER, &lg, sizeof(lg)); } }Then
shutdown(),destroy(), andclose()each callimpl->clear_user_linger()beforeclose_socket().Also applies to: 915-935
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp` around lines 874 - 898, Extract the repeated linger-clearing logic into a private method on kqueue_socket (e.g. kqueue_socket::clear_user_linger() noexcept) that checks user_set_linger_ and fd_ >= 0 and calls ::setsockopt(fd_, SOL_SOCKET, SO_LINGER, &lg, sizeof(lg)) with l_onoff=0,l_linger=0; then replace the duplicated block in kqueue_socket_service::shutdown() and the similar blocks in kqueue_socket_service::destroy() and kqueue_socket::close() to simply call impl->clear_user_linger() before impl->close_socket(); keep the helper noexcept and name it so it clearly indicates it clears the user-set linger option.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp`:
- Around line 996-1000: The close path currently calls
kqueue_socket::close_socket() directly and therefore doesn't clear SO_LINGER
before the fd is closed (unlike destroy() and shutdown()), which allows an RST
to be sent; modify kqueue_socket_service::close to clear the socket's SO_LINGER
option on the underlying fd (the same linger-clearing logic used in
destroy()/shutdown()) before delegating to kqueue_socket::close_socket(), or
factor that linger-clearing code into a small helper (e.g.,
clear_socket_linger(fd) used by destroy(), shutdown(), and close()) so all three
paths clear SO_LINGER prior to closing.
---
Nitpick comments:
In `@include/boost/corosio/native/detail/kqueue/kqueue_socket_service.hpp`:
- Around line 874-898: Extract the repeated linger-clearing logic into a private
method on kqueue_socket (e.g. kqueue_socket::clear_user_linger() noexcept) that
checks user_set_linger_ and fd_ >= 0 and calls ::setsockopt(fd_, SOL_SOCKET,
SO_LINGER, &lg, sizeof(lg)) with l_onoff=0,l_linger=0; then replace the
duplicated block in kqueue_socket_service::shutdown() and the similar blocks in
kqueue_socket_service::destroy() and kqueue_socket::close() to simply call
impl->clear_user_linger() before impl->close_socket(); keep the helper noexcept
and name it so it clearly indicates it clears the user-set linger option.
|
GCOVR code coverage report https://150.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-17 21:44:48 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #150 +/- ##
========================================
Coverage 82.26% 82.26%
========================================
Files 70 70
Lines 5876 5876
========================================
Hits 4834 4834
Misses 1042 1042 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
2 participants
On macOS, close() with SO_LINGER {1,0} sends RST instead of FIN. RST does not reliably trigger EV_EOF on the peer's kqueue, causing pending reads to hang. The previous workaround called shutdown(SHUT_WR) before close to force a FIN.
Replace this with the approach used by Asio's kqueue reactor: clear SO_LINGER before close so the kernel sends FIN naturally. Also skip the explicit EV_DELETE (deregister_descriptor) on both sockets and acceptors, relying on kqueue's automatic cleanup when the fd is closed — matching Asio's deregister_descriptor(closing=true) path.
Update socket throughput benchmark to match asio.
Summary by CodeRabbit
Bug Fixes