http: highWaterMark not respected when agent reuses socket with different HWM

[trivenay]

Problem

When an http.Agent reuses a pooled socket whose writableHighWaterMark differs from the new request's highWaterMark, two issues arise:

1. The user's highWaterMark is silently ignored once Path A (socket connected) takes over
2. Under TCP backpressure conditions, a permanent deadlock can occur

This was identified during review of #64653 by @pimterry (comment).

Reproduction: highWaterMark not respected

const http = require('http');

const server = http.createServer((req, res) => {
  req.resume();
  req.on('end', () => res.end('ok'));
}).listen(0, () => {
  const port = server.address().port;
  const agent = new http.Agent({ keepAlive: true });

  // Request A: creates socket with HWM=1MB
  http.request({ port, method: 'POST', agent, highWaterMark: 1024 * 1024 }, (res) => {
    res.resume();
    res.on('end', () => {
      setTimeout(() => {
        // Request B: HWM=10KB, gets reused socket (HWM=1MB)
        const req = http.request({ port, method: 'POST', agent, highWaterMark: 10 * 1024 });
        req.on('socket', () => {
          setTimeout(() => {
            console.log('Requested HWM: 10KB');
            console.log('Socket HWM:', req.socket.writableHighWaterMark); // 1MB!
            const r = req.write(Buffer.alloc(50 * 1024));
            console.log('write(50KB):', r, '— expected false, got true');
            req.end();
            req.on('response', (r2) => { r2.resume(); r2.on('end', () => server.close()); });
          }, 10);
        });
      }, 100);
    });
  }).end('x');
});

User requests 10KB backpressure but gets 1MB from the reused socket. write(50KB) returns true when it should return false.

Reproduction: deadlock

For the deadlock, three conditions must align:

1. Write size > OM's kHighWaterMark (so write() returns false, kNeedDrain set)
2. Write size < reused socket's writableHighWaterMark (so socket.write() returns true, socket never emits drain)
3. Write size > kernel TCP send buffer (so socket.writableLength > 0 when _flush() checks, preventing drain emission at line 1204)

const http = require('http');

const server = http.createServer((req, res) => {
  setTimeout(() => { req.resume(); req.on('end', () => res.end('ok')); }, 30000);
}).listen(0, () => {
  const port = server.address().port;
  const agent = new http.Agent({ keepAlive: true });

  http.request({ port, method: 'POST', agent, highWaterMark: 10 * 1024 * 1024 }, (res) => {
    res.resume();
    res.on('end', () => {
      setTimeout(() => {
        const req = http.request({ port, method: 'POST', agent });
        const r = req.write(Buffer.alloc(2 * 1024 * 1024));
        if (!r) {
          setTimeout(() => { console.error('DEADLOCK'); process.exit(1); }, 15000);
          req.on('drain', () => req.end());
        } else {
          req.end();
        }
      }, 100);
    });
  }).end('x');
});

sysctl -w net.ipv4.tcp_wmem="4096 16384 65536"
node repro.js  # DEADLOCK

Root cause

In _http_outgoing.js, _flush() emits drain only if this.writableLength === 0 (line 1204). When a reused socket has a higher HWM than the OM, the socket accepts all data without backpressure (socket.write() returns true), but socket.writableLength may remain > 0 if the kernel TCP send buffer can't accept it all instantly. The socket never emits its own drain (was never backpressured), and _flush() can't emit drain either (writableLength !== 0). Result: permanent deadlock.

Proposed fix (two parts)

Part 1: Fix drain emission in _flush() (required)

When _flush() runs and kNeedDrain is set, the OM's own buffer (outputData) has been fully written to the socket. The backpressure signal that originally set kNeedDrain (from Path B) is resolved — the data moved to the socket. Even if socket.writableLength > 0, the OM should emit drain because from the OM's perspective the buffer has been handed off.

This prevents the deadlock regardless of where the socket came from (default agent, custom agent, or createConnection).

Part 2: Don't reuse socket with mismatched HWM in http.Agent (nice to have)

In Agent.prototype.addRequest(), if the pooled socket's writableHighWaterMark differs from options.highWaterMark, destroy the socket and create a fresh one. This ensures HWM is respected for all users of the built-in http.Agent (including globalAgent and new http.Agent()).

For requests to the same host:port, it's rare that different highWaterMark values would be used, so socket reuse still happens for the vast majority of connections.

This doesn't help custom agents or createConnection (they bypass http.Agent), but Part 1's drain fix covers those cases by preventing deadlock.

Discussion from #64653

@pimterry correctly pointed out that option 2 alone isn't sufficient because custom agents and createConnection can return sockets with any HWM. His suggestion (comment):

• Treat highWaterMark as best-effort for the socket path
• Document that socket.connect options depend on agent/createConnection behavior
• Fix the drain logic so deadlocks don't occur regardless of HWM mismatch

I agree the drain fix (Part 1) is the must-have. Part 2 is defense-in-depth for the common case where http.Agent is used.

Refs: #64653, #62936