net: runtime-deprecate Server.prototype._listen2

Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
PR-URL: #64794
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Author: araujogui

doc/api/deprecations.md

@@ -4697,6 +4697,23 @@ underlying stream are emitted from `req`. On the write-side you can use
 `res.writableFinished` to confirm whether the response was written
 successfully before the response closed.
 
+### DEP0208: `Server.prototype._listen2`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64794
+    description: Runtime deprecation.
+-->
+
+Type: Runtime
+
+`net.Server.prototype._listen2` is an undocumented alias for an internal
+function that sets up the listening handle. It is kept only so that code
+replacing it keeps being called by [`server.listen()`][], and it will be
+removed in a future version of Node.js. Use [`server.listen()`][] instead of
+calling or overriding `_listen2`.
+
 [DEP0142]: #dep0142-repl_builtinlibs
 [DEP0156]: #dep0156-aborted-property-and-abort-aborted-event-in-http
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
@@ -4814,6 +4831,7 @@ successfully before the response closed.
 [`response.writableEnded`]: http.md#responsewritableended
 [`response.writableFinished`]: http.md#responsewritablefinished
 [`script.createCachedData()`]: vm.md#scriptcreatecacheddata
+[`server.listen()`]: net.md#serverlisten
 [`setInterval()`]: timers.md#setintervalcallback-delay-args
 [`setTimeout()`]: timers.md#settimeoutcallback-delay-args
 [`socket.bufferSize`]: net.md#socketbuffersize

lib/net.js

@@ -133,6 +133,7 @@ const {
 const { isUint8Array } = require('internal/util/types');
 const { queueMicrotask } = require('internal/process/task_queues');
 const {
+  deprecate,
   guessHandleType,
   isWindows,
   kEmptyObject,
@@ -2357,7 +2358,27 @@ function setupListenHandle(address, port, addressType, backlog, fd, flags) {
                              this);
 }
 
-Server.prototype._listen2 = setupListenHandle;  // legacy alias
+// Legacy alias for `setupListenHandle`, kept around only because it is an
+// undocumented monkeypatch point. Nothing in core calls it unless it has been
+// overridden, see `callSetupListenHandle`.
+const legacyListen2 = deprecate(
+  setupListenHandle,
+  'Server.prototype._listen2 is deprecated. Use Server.prototype.listen() instead.',
+  'DEP0208');
+Server.prototype._listen2 = legacyListen2;
+
+// Set up the listen handle, going through `_listen2` when userland replaced it
+// so that the monkeypatch keeps taking effect (DEP0208). Servers that did not
+// touch `_listen2` must not trigger the deprecation warning.
+function callSetupListenHandle(server, address, port, addressType, backlog,
+                               fd, flags) {
+  if (server._listen2 !== legacyListen2) {
+    server._listen2(address, port, addressType, backlog, fd, flags);
+    return;
+  }
+  FunctionPrototypeCall(setupListenHandle, server, address, port, addressType,
+                        backlog, fd, flags);
+}
 
 // A listening TCP Server can be transferred to another thread, which moves the
 // underlying listening socket (and its pending accept queue) to that thread's
@@ -2431,9 +2452,8 @@ function listenInCluster(server, address, port, addressType,
 
   if (cluster.isPrimary || exclusive) {
     // Will create a new handle
-    // _listen2 sets up the listened handle, it is still named like this
-    // to avoid breaking code that wraps this method
-    server._listen2(address, port, addressType, backlog, fd, flags);
+    callSetupListenHandle(server, address, port, addressType, backlog, fd,
+                          flags);
     return;
   }
 
@@ -2467,9 +2487,8 @@ function listenInCluster(server, address, port, addressType,
     }
     // Reuse primary's server handle
     server._handle = handle;
-    // _listen2 sets up the listened handle, it is still named like this
-    // to avoid breaking code that wraps this method
-    server._listen2(address, port, addressType, backlog, fd, flags);
+    callSetupListenHandle(server, address, port, addressType, backlog, fd,
+                          flags);
   }
 }
 

test/parallel/test-net-listen-handle-in-cluster-2.js

@@ -14,7 +14,8 @@ if (cluster.isPrimary) {
   const handle = new TCP(TCPConstants.SOCKET);
   const errno = handle.bind('0.0.0.0', 0);
   assert.strictEqual(errno, 0);
-  // Execute _listen2 instead of cluster._getServer in listenInCluster
+  // Set up the listen handle directly instead of going through
+  // cluster._getServer in listenInCluster
   net.createServer().listen(handle, common.mustCall(() => {
     process.exit(0);
   }));

test/parallel/test-net-server-listen2-deprecation.js

@@ -0,0 +1,27 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+// `Server.prototype._listen2` is a deprecated alias for the internal function
+// that sets up the listening handle (DEP0208).
+
+common.expectWarning(
+  'DeprecationWarning',
+  'Server.prototype._listen2 is deprecated. Use Server.prototype.listen() instead.',
+  'DEP0208');
+
+// Listening without touching `_listen2` must not emit the warning.
+const server = net.createServer();
+server.listen(0, common.mustCall(() => {
+  server.close(common.mustCall(() => {
+    // Calling the alias directly emits the warning. It still sets up the
+    // handle, so the server ends up listening.
+    const legacy = net.createServer();
+    legacy.on('listening', common.mustCall(() => {
+      assert.strictEqual(legacy.listening, true);
+      legacy.close();
+    }));
+    legacy._listen2(null, 0, 4, undefined, undefined, 0);
+  }));
+}));

test/parallel/test-net-server-listen2-monkeypatch.js

@@ -0,0 +1,25 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+// Overriding the deprecated `Server.prototype._listen2` alias (DEP0208) is
+// still honored by `server.listen()`.
+
+common.expectWarning(
+  'DeprecationWarning',
+  'Server.prototype._listen2 is deprecated. Use Server.prototype.listen() instead.',
+  'DEP0208');
+
+const original = net.Server.prototype._listen2;
+net.Server.prototype._listen2 = common.mustCall(function(...args) {
+  assert.strictEqual(this, server);
+  assert.deepStrictEqual(args, [null, 0, 4, 0, undefined, 0]);
+  return original.apply(this, args);
+});
+
+const server = net.createServer();
+server.listen(0, common.mustCall(() => {
+  net.Server.prototype._listen2 = original;
+  server.close();
+}));