Repository navigation
fix(libkernel): return ENOTCONN from getpeername once a socketpair peer closed - #8344
oneandonlydean merged 3 commits into
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Linux data point for
|
c4450bc to
aeedce4
Compare
aeedce4 to
8e992c3
Compare
github-actions
Bot
added
needs rebase
…er closed getpeername on a socketpair end returned the unnamed AF_UNIX address even after the other end was closed. In FreeBSD 11, closing an end runs uipc_detach, whose unp_disconnect and unp_drop clear SS_ISCONNECTED on both stream and datagram ends, and kern_getpeername then fails with ENOTCONN. Each end now keeps a weak reference to its peer's socket, which expires when the peer's last descriptor, duplicates included, is closed. shutdown leaves the ends connected, as soshutdown does.
8e992c3 to
0ca2e0b
Compare
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the peer-lifetime change and its focused tests. Holding only a weak_ptr to the peer lets dup share the same socket object while ensuring the peer expires after the last descriptor owner closes; shutdown leaves the object alive, matching the behavior described. The tests cover stream shutdown/close, untouched getpeername outputs, and a datagram peer retained by a duplicated descriptor. The PR reports a Windows control that fails on main and passes with the fix, plus WSL2 results; I have not rerun the native tests locally.
One integration item remains: against current upstream main 16319642a4c84e861188b75583ce853dfbe0a101, tools/check_main_sync.py --base upstream/main --head upstream/pr-8344-current reports 1 commit ahead / 10 behind (merge base 9d6adeb55), with a clean merge. Please merge current main and rerun guest_socket_pair plus the focused socket tests. Preserve the adjacent null-length ordering change from open #7502 in Udp.cpp; that PR changes neighboring lines but has a separate behavior. This request is for branch freshness and integration, not PR volume.
|
@MarcZX10 Thanks. I merged current #7502's null-length change in
|
|
@oneandonlydean Thanks. The local Windows run that passed was on this exact head 8f66466: 40 of 40 socket, |
The requested integration is done: head 8f66466 merges current main, keeps the adjacent null-length handling in Udp.cpp, and the full Linux and Windows run (40101561852) is green with guest_socket_pair passing on both.
…rname-enotconn # Conflicts: # core/libs/tests/GuestSocketPair.cpp
|
Merged |
What
getpeernameon asocketpairend kept returning the unnamedAF_UNIXaddress after the other end was closed. On FreeBSD 11, closing an end runsuipc_detach, whoseunp_disconnect(datagram) andunp_drop(stream) clearSS_ISCONNECTEDon the remaining end (sys/kern/uipc_usrreq.c), andkern_getpeernamethen fails withENOTCONN(sys/kern/uipc_syscalls.c).Each pair end now holds a
std::weak_ptrto the other end's socket. The reference expires when the peer's last descriptor is closed,dup'd copies included, andgetpeernamethen fails withENOTCONN(57).shutdownleaves the ends connected, assoshutdowndoes. The getpeername clause is removed from the socketpair entry indocs/dev/debt/libkernel.md, and "like on every guest socket" becomes "as on every guest socket" to keep the entry under the 1000-character limit.guest_socket_pairnow checks:shutdown(SHUT_WR)still reports the unnamed peer;close,getpeernamefails withENOTCONNand leaves the address and length untouched, whilegetsocknamestill succeeds;dup'd copy stays open keeps the pair connected, and closing the copy disconnects it.Tested
Windows 11, WinLibs MinGW-w64 GCC 15.2.0 r7, CMake + Ninja, Release,
-DBUILD_TESTING=ON, rebased onmain9d6adeb.guest_socket_pairwithmain'sUdp.cppfails (exit 0xc0000409 from the failedRequire); with this PR it passes.-R "socket|dup|descriptor|fcntl|pipe|poll|select|recvmsg" -E "^agc_": 41 of 41 passed after the rebase onto 47fa6ee;guest_socket_pairpasses again after the rebases onto 11f976b and 9d6adeb, whose only conflict each time was the socketpair debt entry.main6cd36e6 (head 8f66466, clean merge): ctest-R "socket|recvmsg|net_dontwait|^guest_poll$", 40 of 40 pass when run serially, includingguest_socket_pairandguest_recvmsg_memory. Withmain'sUdp.cppon that tree,guest_socket_pairfails again (exit 0xc0000409) andguest_recvmsg_memorystill passes.python tools/check_conventions.py --base origin/mainreports no errors. Its one warning is the length of the socketpair debt entry, 1382 characters onmain, which this PR cuts to 1278.guest_socket_pairandguest_recvmsg_memorypass, andguest_socket_pairfails withmain'sUdp.cpp(comment below). Not run on bare-metal Linux.Overlap:
getpeername, so whichever merges second needs a one-line rebase.inodemember toSocketnext to this PR's member. This PR goes first, and fix(libkernel): report NODEV and a per-socket inode from fstat on a socketpair end #10621 declaresDepends on: #8344.getpeernamesentence.tools/pr_overlap.pycould not compare some PRs. I checked those diffs by hand: none changesgetpeernameor the socketpair debt entry.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)