[harmon25]

Distinguish transient send backpressure from a closed connection in socket:send/2,3, and wait for write-readiness with a proper write select instead of leaking {error, eagain} or {ok, Rest} to callers.

Ran into this building httpd, a fork/standalone implementation of atomvm_lib/httpd: large or many subsequent requests failed with the socket closing early, because lwIP ERR_MEM backpressure was reported as {error, closed}.

Commits

1. Fix socket send reporting backpressure as closed: lwIP ERR_MEM / BSD EAGAIN|EWOULDBLOCK is now {error, eagain}, and a closed connection is {error, closed} instead of {ok, Data}.
2. Add enif_select_write and ERL_NIF_SELECT_CANCEL: enif_select_write was declared but never implemented. A select event now keeps independent read and write state (ref, message, pid), so a resource can have a pending read and a pending write select at the same time. ERL_NIF_SELECT_CANCEL (with the *_CANCELLED flags) cancels one direction and keeps the other armed. Also fixes the ref ticks computation. C tests in tests/test-enif.c.
3. otp_socket: Add nif_select_write/2: BSD uses enif_select_write; lwIP uses tcp_sent/tcp_poll and tcp_sndbuf/tcp_sndqueuelen. A socket has one selecting process and monitor per direction, like resources.c: select replaces only its own direction, close and select_stop handle both, socket_down releases only the dead owner's direction (and no longer aborts the lwIP connection, like BSD), aborts go to each direction's own pid, and the lwIP handlers notify the owner of their direction. The selecting state is initialized by one helper shared by open and accept. socket:nif_select_read_stop/1 stops only the read direction when another process is selecting for write, and is used by the read/accept error cleanup paths so that a failed read does not disown a blocked writer. On lwIP, tcp_sent events are only queued while a write select is pending, lwip_tcp_can_send is called with the lwIP lock held and only on connected sockets, and every pcb callback is detached before a pcb is closed so none can run against a freed resource.
4. socket: Retry sends on backpressure: socket:send/2 and sendto/3 wait on nif_select_write/2 and retry, so callers get ok | {error, Reason} (or {error, {Reason, RestData}} after a partial send, as in OTP); empty payloads still go through the NIF like before.
5. Change socket:send callers to the new semantics: ssl waits for write-readiness instead of busy looping on want_write; socket_dist_controller terminates the connection when socket:send/2 fails; dead {ok, Rest} branches are removed.
6. Add tests: send backpressure with write select; closing an accepted socket selected by another process (valgrind flags the uninitialized state without the fix); close with a blocked reader and a blocked writer; killing the writer leaves the reader working.
7. Document socket write select in the changelog.

Known limitations

• On lwIP, a peer reset (tcp_err_cb) does not wake a process blocked in socket:send/2 or recv. It only answers a pending connect, as before. Tracked in lwIP sockets: a peer reset does not wake processes blocked in socket:send/2 or recv #2432.
• socket:send/2 now blocks, so socket_dist_controller can block on a peer that stops reading and miss the tick that would detect it: socket_dist_controller: sends can block the controller and hide a dead peer #2424.

Testing

• test-enif (including the new ERL_NIF_SELECT_CANCEL tests) and the test_estdlib suite on generic_unix, including test_tcp_socket under valgrind. test_serial_dist_socat fails in the same way on a plain release-0.7 build here (socat/pty environment).
• New tests: a reader cleaning up after the peer half-closes must not disown a blocked writer (fails without nif_select_read_stop), and empty send/2/sendto/3 payloads (also run on BEAM).
• The new socket tests fail on the single owner implementation (the reader hangs) and pass now.
• Every commit builds and test-enif passes on each. ESP32 (ESP-IDF 5.5.4) build, and the lwIP code path syntax-checked against ESP-IDF's lwIP headers: not run on RP2 hardware.
• Earlier on real ESP32-S3 hardware with atomvm_httpd: 3 concurrent slow readers (curl --limit-rate 1k against a 64KB response) logged only eagain retries (never closed) and every response completed with the full body. That run predates the review changes below.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[petermm]

thank you for pushing the httpd side!

this is obviously correct - however it does increase the leakage of underlying implementation, and now any caller will also need to implement their own unique retry mechanism (gen_tcp_socket.erl, socket_dist_controller.erl, ssl.erl - and atomvm_lib httpd) - and is it 5ms with exponential backoff or xyz - and we end up with complexity, duplicated and slow (sleeping) code..

I believe the solution is to implement nif_select_write/2, with proper write-select and no sleeping/guessing, eg. handle the happy path out of the box, similar to otp:

see https://github.com/petermm/AtomVM/tree/nif_select_write/2 (based on this PR)

[harmon25]

Thanks @petermm! I agree, that sounds like a better approach.

Should we continue from your branch, or should i incorporate those changes here?

[harmon25]

@petermm Thanks for the review and for prototyping this on nif_select_write/2 — agreed this is the right fix, and I've implemented it on top of this branch (6 new commits, keeping the existing eagain/closed distinction as the low-level primitive underneath):

1. enif_select_write — was declared in erl_nif.h but never implemented or called anywhere. Implemented it, and reworked enif_select/enif_select_read/enif_select_write so a resource can hold independent pending read and write selects (each with its own ref/message/pid) on the same event at the same time, instead of one direction silently overwriting the other's state (needed since a socket can have a permanent recv select active at the same time as a transient send-backpressure select, e.g. socket_dist_controller). Also fixed a latent ref-ticks bug found along the way (using term_is_local_reference(ref) instead of ref != UNDEFINED_ATOM). Added C-level unit test coverage for this in tests/test-enif.c (there was none before for enif_select at all).
2. otp_socket.c — wired up nif_select_write/2 for both BSD (enif_select_write) and lwIP (new tcp_sent/tcp_poll callbacks + a SocketStateSelectingWrite state, using tcp_sndbuf/tcp_sndqueuelen to detect room again).
3. socket:send/2,3 — now retries partial sends and {error, eagain} internally by waiting on nif_select_write, so callers just see ok | {error, Reason} — no more leaking {ok, Rest}/{error, eagain}.
4. ssl.erl — replaced the 4 want_write busy-loops with a real wait-for-writable.
5. socket_dist_controller.erl — now actually terminates the connection on socket:send/2 errors instead of silently ignoring them (tick + outgoing data loop).
6. Added a real backpressure test to test_tcp_socket.erl (fills the send buffer via raw nif_send/2 until genuine EAGAIN, then verifies nif_select_write/2 correctly wakes up once the peer drains, and that socket:send/2 completes transparently afterward).

Verified: test-enif, the full test_estdlib suite (including the new backpressure test) on generic_unix, and a full ESP32 (esp32s3) build via ESP-IDF 5.5.4.

One thing worth calling out from writing the test: I initially had it drain only a small chunk (64KB) of the peer's backlog and expected the socket to become writable again, but that hung — traced it down (gdb backtraces + an independent raw C poll(2) repro, unrelated to any AtomVM code) to the sandbox's kernel/buffer accounting not reliably crossing the write low-water mark on a small partial drain, only on a full drain. Not an AtomVM bug, just a test design fix, but flagging in case it's relevant to anyone else validating this on constrained targets (rp2/esp32) with small buffers — might be worth a similar "drain fully, not partially" pattern if there's ever a device-side test for this.

[bettio]

First of all I apologize for the long wait before this review. Here I have some comments.

I would like to ask you make your branch git history linear without those merges from release-0.7. Ideally your commits should be amended using git commit --fixup and they should sit on top of latest release-0.7. See also GIT_WORKFLOW_AND_HISTORY_STYLE_GUIDE.md

[bettio]

Let's fix the CodeQL error and it is good for merge.

[bettio]

I need to apologize, I think I found other issues just before merging.

[harmon25]

Pushed fixes for all six comments, rebased on the latest release-0.7. Thanks for the careful review. #2422 touches the same area (nif_select_read_stop/1), so whichever lands second will need a small rebase.

[bettio]

I need to apologize here, I have one more review round here. Still I believe we are getting super close.

[harmon25]

Pushed fixes for all six comments, rebased on the latest release-0.7. Thanks again for the careful review.