Skip to content

Commit 56da7ec

Browse files
committed
Merge FEATURE-ci-iocp-close: Honor socket close reference contract
2 parents b5cd6cf + 3876a99 commit 56da7ec

2 files changed

Lines changed: 83 additions & 7 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# FEATURE-ci-iocp-close
2+
3+
Status: Complete (local implementation, Groundwork validation and finalization).
4+
Native Windows results remain CI / v3 PR owned; no release readiness claimed.
5+
6+
Scope: repair the IOCP socket retirement call rejected by the Windows compiler.
7+
Base: v3 at `b5cd6cf06e39a1b0727c2614e5236ff00a59dbf1`.
8+
9+
- [x] Create an isolated task worktree using the Groundwork identity helper.
10+
- [x] Read the actual native Windows compile failure and verify its source is
11+
unchanged at the task base.
12+
- [x] Pass the registration socket lvalue to the shared close helper, preserving
13+
handle invalidation, map erasure, cancellation and completion lease lifetime.
14+
- [x] Retain the existing native UDP cancellation/reuse test without adding
15+
mirrored source-text or synthetic implementation tests.
16+
- [x] Build and run the focused existing local socket helper suite; run cpplint
17+
on the changed production file and check the diff.
18+
- [x] Retain commands, raw results, original failure and causal assessment on disk.
19+
- [x] Caller: complete Groundwork validation and finalization.
20+
Three focused reviewers approved with no findings; matching evidence reused.
21+
- [ ] CI / v3 PR: obtain fresh native Windows compile and IOCP runtime results.
22+
23+
## Failure evidence and causal assessment
24+
25+
The original native Windows package job `120361073253` on `808f7fa` failed at
26+
`src/detail/io_iocp_backend.cpp:260`: the temporary returned by `std::exchange`
27+
cannot bind to `close_socket(native_socket_t&)`. The full original log is
28+
retained at `.debug/FEATURE-ci-iocp-close/windows-job-120361073253.log` in the
29+
main repository; lines 751–763 contain the compiler diagnostic. A recorded
30+
`git diff 808f7fa b5cd6cf -- src/detail/io_iocp_backend.cpp` is empty, tying that
31+
source-bound RED to this task base. The accepted plan uses the real compile
32+
failure as RED for this compile-only repair.
33+
34+
`close_socket` already copies the closing handle, sets the caller's reference
35+
to `k_invalid_socket`, then closes the copied handle. Passing `record->socket`
36+
directly honors that contract for both stream and datagram registrations. A
37+
retained completion packet's eventual registration destructor sees the invalid
38+
handle, so it cannot close a socket that reused the old value. The now redundant
39+
datagram branch is removed; the lifetime comment remains. Connection map erasure
40+
and `CancelIoEx` still precede the helper call, with the same mutex and packet
41+
ownership. No other source or public interface changed.
42+
43+
## Local verification and limits
44+
45+
From the isolated task worktree, with `EVIDENCE` denoting the absolute main
46+
repository directory `.debug/FEATURE-ci-iocp-close`:
47+
48+
```sh
49+
g++ -std=c++20 -O0 -g -Wall -Wextra -pthread -DHTTPSERVER_COMPILATION \
50+
-Isrc -Itest test/unit/io_poll_sys_listener_test.cpp \
51+
src/detail/io_poll_sys.cpp src/net/address.cpp src/detail/net_address.cpp \
52+
-o "$EVIDENCE/io_poll_sys_listener"
53+
timeout 30s "$EVIDENCE/io_poll_sys_listener"
54+
cpplint src/detail/io_iocp_backend.cpp
55+
git diff --check
56+
```
57+
58+
All commands exited 0. The existing listener/helper suite executed 12 tests,
59+
45 successful checks, no failures or skips. It compiles the actual shared socket
60+
helper and uses real local sockets; it supplies local helper coverage, not
61+
Windows IOCP proof. cpplint reported no errors. Exact expanded commands, working
62+
directories, output and exit codes are retained in `build-listener.log`,
63+
`run-listener.log`, `cpplint.log` and `diff-check.log` alongside `journal.md`.
64+
65+
The bounded local capability check found no Windows crosscompiler or SDK.
66+
Native Windows compilation and
67+
`udp_cancelled_packet_keeps_native_storage_and_reuse_generation` execution remain
68+
incomplete until CI runs them. No substitute native pass is claimed. No broad
69+
baseline or unrelated repairs were run; previous task evidence and the unrelated
70+
bauth worktree remain unchanged.
71+
72+
## Implementation notes
73+
74+
General setup uses `./bootstrap`, an out-of-tree configure/build, and
75+
`make check`; this task needed only the dependency-free focused compile above.
76+
Tests use `test/littletest.hpp`, `LT_BEGIN_AUTO_TEST` and `LT_CHECK` assertions.
77+
Private implementations live in `src/detail`, owning headers in
78+
`src/httpserver/detail`; the socket helper takes a mutable lvalue reference and
79+
invalidates before closing. Git metadata writes required sandbox escalation.
80+
Task-local setup and test memory is retained in the evidence directory.

‎src/detail/io_iocp_backend.cpp‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,9 @@ void io_iocp_backend::retire_locked(std::uint64_t id, completions& done) {
254254
const auto record = found->second;
255255
connections_.erase(found); // invalidate before socket reuse
256256
::CancelIoEx(reinterpret_cast<HANDLE>(record->socket), nullptr);
257-
if (record->datagram) {
258-
// The completion lease survives closure; do not let its destructor
259-
// close a descriptor reused by a newly adopted UDP socket.
260-
pollsys::close_socket(std::exchange(record->socket, pollsys::k_invalid_socket));
261-
} else {
262-
pollsys::close_socket(record->socket);
263-
}
257+
// close_socket invalidates the handle before closing; a surviving
258+
// completion lease must not close a descriptor reused by a new socket.
259+
pollsys::close_socket(record->socket);
264260
}
265261
std::vector<std::shared_ptr<op_state>> swept;
266262
for (const auto& entry : pending_) {

0 commit comments

Comments
 (0)