Skip to content

Commit 02a12d9

Browse files
committed
FEATURE-ci-iocp-close: Honor the socket close lvalue contract
Use the shared invalidating close helper for both IOCP socket kinds while preserving cancellation and packet lease ordering. Retain native compiler RED and focused local helper, lint and diff evidence; Windows compile and runtime remain CI-owned.
1 parent b5cd6cf commit 02a12d9

2 files changed

Lines changed: 82 additions & 7 deletions

File tree

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