fix(net/tls) we need to call end when we got FIN if allowHalfOpen is …

…false (#13212)

Co-authored-by: Jarred Sumner <[email protected]>

src/js/node/net.ts

@@ -68,13 +68,13 @@ const bunSocketServerOptions = Symbol.for("::bunnetserveroptions::");
 const bunSocketInternal = Symbol.for("::bunnetsocketinternal::");
 const bunTLSConnectOptions = Symbol.for("::buntlsconnectoptions::");
 
-function closeNT(self) {
-  self.emit("close");
-}
 function endNT(socket, callback, err) {
   socket.end();
   callback(err);
 }
+function closeNT(callback, err) {
+  callback(err);
+}
 
 var SocketClass;
 const Socket = (function (InternalSocket) {
@@ -109,7 +109,7 @@ const Socket = (function (InternalSocket) {
         queue.push(buffer);
       },
       drain: Socket.#Drain,
-      end: Socket.#Close,
+      end: Socket.#End,
       error(socket, error) {
         const self = socket.data;
         if (!self) return;
@@ -187,17 +187,37 @@ const Socket = (function (InternalSocket) {
       binaryType: "buffer",
     };
 
+    static #End(socket) {
+      const self = socket.data;
+      if (!self) return;
+      self.#ended = true;
+      const queue = self.#readQueue;
+      if (queue.isEmpty()) {
+        if (self.push(null)) {
+          return;
+        }
+      }
+      queue.push(null);
+    }
     static #Close(socket) {
       const self = socket.data;
       if (!self || self.#closed) return;
       self.#closed = true;
       //socket cannot be used after close
       self[bunSocketInternal] = null;
-      const queue = self.#readQueue;
-      if (queue.isEmpty()) {
-        if (self.push(null)) return;
+      const finalCallback = self.#final_callback;
+      if (finalCallback) {
+        self.#final_callback = null;
+        finalCallback();
+        return;
+      }
+      if (!self.#ended) {
+        const queue = self.#readQueue;
+        if (queue.isEmpty()) {
+          if (self.push(null)) return;
+        }
+        queue.push(null);
       }
-      queue.push(null);
     }
 
     static #Drain(socket) {
@@ -322,6 +342,8 @@ const Socket = (function (InternalSocket) {
     bytesRead = 0;
     bytesWritten = 0;
     #closed = false;
+    #ended = false;
+    #final_callback = null;
     connecting = false;
     localAddress = "127.0.0.1";
     #readQueue = $createFIFO();
@@ -609,14 +631,21 @@ const Socket = (function (InternalSocket) {
     }
 
     _destroy(err, callback) {
-      const socket = this[bunSocketInternal];
-      socket && process.nextTick(endNT, socket, callback, err);
+      process.nextTick(closeNT, callback, err);
     }
 
     _final(callback) {
-      this[bunSocketInternal]?.end();
-      callback();
-      process.nextTick(closeNT, this);
+      const socket = this[bunSocketInternal];
+      // already closed call destroy
+      if (!socket) return callback();
+
+      if (this.allowHalfOpen) {
+        // wait socket close event
+        this.#final_callback = callback;
+      } else {
+        // emit FIN not allowing half open
+        process.nextTick(endNT, socket, callback);
+      }
     }
 
     get localFamily() {

test/js/node/net/node-net.test.ts

@@ -490,3 +490,36 @@ it("socket should keep process alive if unref is not called", async () => {
   });
   expect(await process.exited).toBe(1);
 });
+
+it("should not hang after FIN", async () => {
+  const net = require("node:net");
+  const { promise: listening, resolve: resolveListening, reject } = Promise.withResolvers();
+  const server = net.createServer(c => {
+    c.write("Hello client");
+    c.end();
+  });
+  try {
+    server.on("error", reject);
+    server.listen(0, () => {
+      resolveListening(server.address().port);
+    });
+    const process = Bun.spawn({
+      cmd: [bunExe(), join(import.meta.dir, "node-fin-fixture.js")],
+      stderr: "inherit",
+      stdin: "ignore",
+      stdout: "inherit",
+      env: {
+        ...bunEnv,
+        PORT: ((await listening) as number).toString(),
+      },
+    });
+    const timeout = setTimeout(() => {
+      process.kill();
+      reject(new Error("Timeout"));
+    }, 1000);
+    expect(await process.exited).toBe(0);
+    clearTimeout(timeout);
+  } finally {
+    server.close();
+  }
+});