[PATCH v5 http-server 0/2] fix pveproxy OOM in websocket and spice proxy handlers

[Kefu Chai <k.chai@proxmox.com>]

see v2's cover letter [1] for the problem description and the approach.

v4 does not fix the reported bug. On a TLS handle, AnyEvent keeps
reading even after on_read is cleared, so the websocket client side
kept filling rbuf until it hit rbuf_max, and the connection was dropped
with ENOSPC. That is the direction PDM migration sends its data in.

Patch 1 is v4 with the fixes below. Patch 2 is new: when one side of a
proxied connection closes, the proxy dropped the data still queued for
the other side. v4 tried to address this with the drain in on_eof, but
the data is lost in client_do_disconnect(), which shuts the socket down
before its wbuf is written. Patch 2 waits for the queued data to be
written before disconnecting. It uses the same timeout rule as patch 1:
60 seconds when waiting for the client, none when waiting for the
backend. It is a separate patch because master has the same problem,
and it can be reviewed on its own.

Changes to patch 1 since v4 [2]:

* call stop_read() when pausing, so the pause also works on the TLS
  client handle.

* set a 60 second write timeout when the client stops reading. Without
  it, such a client kept the connection open forever, because the proxy
  handles have no timeout and a paused handle does not notice EOF.
  There is no timeout when the backend stops reading. The backend is a
  local process, and closing its connection early drops the data still
  queued for it. For a remote disk import in raw+size format, dd then
  reaches EOF and the import succeeds with the end of the disk missing.

* on a WebSocket Close frame, disconnect once the queued data is
  written, the same way finish_response() does, instead of calling
  push_shutdown(). push_shutdown() only half-closed the socket and
  dropped a paused backend reader, so the backend was never read again.

* drop the rbuf drain in on_eof, together with handle_proxy_eof(). A
  paused handle has no read watcher, so EOF only shows up after the
  reader has resumed and emptied rbuf, and there is never anything left
  to drain. This also removes the eval split added in v4.

* do not call the previous on_drain twice when resuming, since
  on_drain() already calls it when restoring it.

* use abort_request() (was handle_proxy_error()) for the accept-time
  client handle as well, define the 640 KB limit once as
  $limit_proxy_wbuf, and move the push-and-pause code of the four proxy
  readers into proxy_forward().

I tested master, v4 and this series with an out-of-tree setup (not part
of this series): the real server with TLS and a stub rest_handler, a
websocket client, and a plain TCP backend.

* with the client sending 256 MB and the backend reading at 32 MB/s,
  master grew by 246 MB and v4 dropped the connection after about
  7 MB. v5 finished with the data intact and no memory growth.

* when the client stopped reading, v5 disconnected after 60 seconds,
  and v4 kept the connection open past the 90 second limit of the test.
  When the backend stopped reading, v5 kept the connection open with
  about 1 MB of memory growth. master buffered whatever arrived in both
  cases (1 GB in the test).

* after a Close frame sent while paused, v5 flushed the queued data and
  closed both sides. v4 closed too, but only because the client gave up
  on a TLS error.

* when the backend closed after sending its data, master lost 252 MB
  of 256 MB. With 64 MB, v4 lost up to 576 KB in 3 of 5 runs and patch
  1 alone up to 624 KB in 4 of 7 runs, and with patch 2 all 7 runs were
  complete. When the client closed without a Close frame, patch 1 alone
  lost up to 691 KB in 3 of 5 runs, and with patch 2 all 7 runs were
  complete. When the client did not read after the backend closed,
  patch 2 disconnected after 60 seconds.

* I also replayed an offline remote disk migration through the real
  proxmox-websocket-tunnel, with stubs for the mtunnel worker and for
  pvesm import. The tunnel ends each forwarded connection with a Close
  frame, so both master and v5 delivered the full disk. With the
  import pausing for 75 seconds when 2 MB were left, v5 still delivered
  the full disk. A version that also timed out on the backend closed
  the connection instead, and the import ended 1.9 MB short without an
  error, which is why v5 has no timeout there.

[1] https://lore.proxmox.com/pve-devel/20260413125650.2569621-1-k.chai@proxmox.com/
[2] https://lore.proxmox.com/pve-devel/20260617122905.3822836-1-k.chai@proxmox.com/

Kefu Chai (2):
  fix #7483: apiserver: add backpressure to proxy handlers
  apiserver: flush queued data before closing a proxied connection

 src/PVE/APIServer/AnyEvent.pm | 189 +++++++++++++++++++++-------------
 1 file changed, 119 insertions(+), 70 deletions(-)

-- 
2.47.3