vmm: prevent serial manager TCP shutdown deadlock

[amphi]

Keep track of the active TCP console client so the serial manager can shut it down before waiting for the worker thread to exit. This prevents a shutdown deadlock where a blocked TCP read keeps VM teardown stuck after migration or console disconnects.

By storing a clone of the active TCP stream and closing it during drop, the read side is forced to wake up and observe the shutdown path instead of waiting forever in the socket read. The worker then exits its epoll loop, the join returns, and VM teardown can finish.

[phip1611]

I think this should be Option<Arc<Mutex<TcpStream>>>. I'm unsure if you need the Arc<Mutex> at all since you seem to have &mut self available on every call site. Would Option<TcpStream> also work?

[amphi]

The variable is shared between the main VMM thread and serial-manager thread, so I think we need at least the Arc, and I'd like to synchronize the access to it, thus the Mutex

[phip1611]

and I'd like to synchronize the access to it, thus the Mutex

Never use a Mutex because you'd like to synchronize access - the compiler either forces you or doesn't complain which means there are no unsynchronized accesses :)

But I assume you wanted to say that Rust complains without Arc<Mutex<>>, hence the Mutex is necessary?

[amphi]

I just tried to remove the Mutex<> part, but yes the compiler complains without it.

[tpressure]

AI comment:

vmm/src/serial_manager.rs:501-524 returns 0 for TCP close, WouldBlock , and Interrupted, but the shared path still calls queue_input_bytes(&input[..count]) at vmm/src/serial_manager.rs:542-547. Zero-length queueing is not harmless: x86 Serial::queue_input_bytes() still calls recv_data() and sets data-ready/interrupt state, and aarch64 Pl011::queue_input_bytes() clears RX-empty state with no byte queued. This can expose spurious readable serial input to the guest. The Ok(0) case existed before, but the PR rewrites this block and adds new zero-count paths. I’d skip queueing when count == 0.

[phip1611]

Code-wise LGTM but what about Thomas' comment?

[amphi]

AI comment:

vmm/src/serial_manager.rs:501-524 returns 0 for TCP close, WouldBlock , and Interrupted, but the shared path still calls queue_input_bytes(&input[..count]) at vmm/src/serial_manager.rs:542-547. Zero-length queueing is not harmless: x86 Serial::queue_input_bytes() still calls recv_data() and sets data-ready/interrupt state, and aarch64 Pl011::queue_input_bytes() clears RX-empty state with no byte queued. This can expose spurious readable serial input to the guest. The Ok(0) case existed before, but the PR rewrites this block and adds new zero-count paths. I’d skip queueing when count == 0.

Oh wtf that was a rebasing mistake I think. I tried making the socket nonblocking first, but then decided that the current solution seems easier and more reliable. I reverted this change.

[phip1611]

After offline discussion. LGTM.