parallel.test-tls-socket-close is flaky

[anonrig]

Test

parallel.test-tls-socket-close

Platform

No response

Console output

not ok 2868 parallel/test-tls-socket-close
  ---
  duration_ms: 220.38800
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:125
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
    
    false !== true
    
        at Immediate._onImmediate (/home/iojs/build/workspace/node-test-commit-linuxone/test/parallel/test-tls-socket-close.js:49:18)
        at process.processImmediate (node:internal/timers:478:21) {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: false,
      expected: true,
      operator: 'strictEqual'
    }
    
    Node.js v21.0.0-pre
  ...

Build links

• https://ci.nodejs.org/job/node-test-commit-linuxone/39972/nodes=rhel8-s390x/console

Additional information

No response

[lpinca]

It seems #49575 did not fully fix it.

[joyeecheung]

I wonder if we really need to check the client socket in the immediate, perhaps it's already enough, say, checking it on process exit?

[joyeecheung]

Not quite sure what the test was going for, perhaps we could also switch to process.nextTick...

EDIT: nope probably not useful

[joyeecheung]

A dumb idea: maybe we can keep checking if the client socket is destroyed in setImmediate, when it's destroyed, log the count of immediates run, assert that the count should be no more then 1, then run a stress test on it to see if it goes beyond one and if it does, what exactly it is. I am fairly certain though that there probably is just no way you can be sure about this count, there's nothing in the API contract for you to tell how long you need to wait after the server socket is destroyed until the client socket is destroyed. So maybe again it's already enough to check on process exit..

[lpinca]

Before Joyee's comments I tried to remove setImmediate() and rely on the 'close' events. If the 'close' event is emitted, then socket.destroy is true.

diff --git a/test/parallel/test-tls-socket-close.js b/test/parallel/test-tls-socket-close.js
index 70af760d53..e661da94f0 100644
--- a/test/parallel/test-tls-socket-close.js
+++ b/test/parallel/test-tls-socket-close.js
@@ -6,6 +6,7 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const tls = require('tls');
 const net = require('net');
+const Countdown = require('../common/countdown');
 const fixtures = require('../common/fixtures');
 
 const key = fixtures.readKey('agent2-key.pem');
@@ -27,6 +28,10 @@ const netServer = net.createServer((socket) => {
   connectClient(netServer);
 }));
 
+const countdown = new Countdown(2, () => {
+  netServer.close();
+});
+
 // A client that connects, sends one message, and closes the raw connection:
 function connectClient(server) {
   const clientTlsSocket = tls.connect({
@@ -42,17 +47,12 @@ function connectClient(server) {
 
       netSocket.destroy();
 
-      setImmediate(() => {
-        assert.strictEqual(netSocket.destroyed, true);
-
-        setImmediate(() => {
-          assert.strictEqual(clientTlsSocket.destroyed, true);
-          assert.strictEqual(serverTlsSocket.destroyed, true);
-
-          tlsServer.close();
-          netServer.close();
-        });
-      });
+      clientTlsSocket.on('close', dec);
+      serverTlsSocket.on('close', dec);
     }));
   }));
 }
+
+function dec() {
+  countdown.dec();
+}

However the 'close' event is not emitted by serverTlsSocket. I'm not sure if this is intentional. Older versions of Node.js have the same behavior but I find it surprising.