From 297c5c555dd21908f85a59a41aa1f43221f62e4c Mon Sep 17 00:00:00 2001
From: Thomas Watson <w@tson.dk>
Date: Wed, 11 Dec 2024 18:33:44 +0100
Subject: [PATCH] [DI] Ensure the tracer doesn't block instrumented app from
 exiting (#4993)

The `MessagePort` objects should be unref'ed (has to be after any
message handler has been attached). Otherwise their handle will keep the
instrumented app running.

Technically there's no need to unref `port1`, but let's just unref
everything show the intent.
---
 .../debugger/target-app/unreffed.js             | 15 +++++++++++++++
 integration-tests/debugger/unreffed.spec.js     | 17 +++++++++++++++++
 integration-tests/debugger/utils.js             |  8 ++++----
 packages/dd-trace/src/debugger/index.js         |  8 ++++++--
 4 files changed, 42 insertions(+), 6 deletions(-)
 create mode 100644 integration-tests/debugger/target-app/unreffed.js
 create mode 100644 integration-tests/debugger/unreffed.spec.js

diff --git a/integration-tests/debugger/target-app/unreffed.js b/integration-tests/debugger/target-app/unreffed.js
new file mode 100644
index 00000000000..3a5353d7399
--- /dev/null
+++ b/integration-tests/debugger/target-app/unreffed.js
@@ -0,0 +1,15 @@
+'use strict'
+
+require('dd-trace/init')
+const http = require('http')
+
+const server = http.createServer((req, res) => {
+  res.end('hello world') // BREAKPOINT
+  setImmediate(() => {
+    server.close()
+  })
+})
+
+server.listen(process.env.APP_PORT, () => {
+  process.send({ port: process.env.APP_PORT })
+})
diff --git a/integration-tests/debugger/unreffed.spec.js b/integration-tests/debugger/unreffed.spec.js
new file mode 100644
index 00000000000..2873d80e190
--- /dev/null
+++ b/integration-tests/debugger/unreffed.spec.js
@@ -0,0 +1,17 @@
+'use strict'
+
+const { assert } = require('chai')
+const { setup } = require('./utils')
+
+describe('Dynamic Instrumentation', function () {
+  const t = setup()
+
+  it('should not hinder the program from exiting', function (done) {
+    // Expect the instrumented app to exit after receiving an HTTP request. Will time out otherwise.
+    t.proc.on('exit', (code) => {
+      assert.strictEqual(code, 0)
+      done()
+    })
+    t.axios.get('/')
+  })
+})
diff --git a/integration-tests/debugger/utils.js b/integration-tests/debugger/utils.js
index c5760a0e9d4..c65bd5c0d88 100644
--- a/integration-tests/debugger/utils.js
+++ b/integration-tests/debugger/utils.js
@@ -18,7 +18,7 @@ module.exports = {
 }
 
 function setup () {
-  let sandbox, cwd, appPort, proc
+  let sandbox, cwd, appPort
   const breakpoint = getBreakpointInfo(1) // `1` to disregard the `setup` function
   const t = {
     breakpoint,
@@ -68,7 +68,7 @@ function setup () {
   }
 
   before(async function () {
-    sandbox = await createSandbox(['fastify'])
+    sandbox = await createSandbox(['fastify']) // TODO: Make this dynamic
     cwd = sandbox.folder
     t.appFile = join(cwd, ...breakpoint.file.split('/'))
   })
@@ -81,7 +81,7 @@ function setup () {
     t.rcConfig = generateRemoteConfig(breakpoint)
     appPort = await getPort()
     t.agent = await new FakeAgent().start()
-    proc = await spawnProc(t.appFile, {
+    t.proc = await spawnProc(t.appFile, {
       cwd,
       env: {
         APP_PORT: appPort,
@@ -97,7 +97,7 @@ function setup () {
   })
 
   afterEach(async function () {
-    proc.kill()
+    t.proc.kill()
     await t.agent.stop()
   })
 
diff --git a/packages/dd-trace/src/debugger/index.js b/packages/dd-trace/src/debugger/index.js
index 3638119c6f1..ea2a36d4d25 100644
--- a/packages/dd-trace/src/debugger/index.js
+++ b/packages/dd-trace/src/debugger/index.js
@@ -57,8 +57,6 @@ function start (config, rc) {
     }
   )
 
-  worker.unref()
-
   worker.on('online', () => {
     log.debug(`Dynamic Instrumentation worker thread started successfully (thread id: ${worker.threadId})`)
   })
@@ -80,6 +78,12 @@ function start (config, rc) {
       rcAckCallbacks.delete(ackId)
     }
   })
+
+  worker.unref()
+  rcChannel.port1.unref()
+  rcChannel.port2.unref()
+  configChannel.port1.unref()
+  configChannel.port2.unref()
 }
 
 function configure (config) {