Skip to content

Commit

Permalink
[DI] Ensure the tracer doesn't block instrumented app from exiting (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
watson authored and rochdev committed Dec 17, 2024
1 parent ddb7467 commit 297c5c5
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
15 changes: 15 additions & 0 deletions integration-tests/debugger/target-app/unreffed.js
Original file line number Diff line number Diff line change
@@ -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 })
})
17 changes: 17 additions & 0 deletions integration-tests/debugger/unreffed.spec.js
Original file line number Diff line number Diff line change
@@ -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('/')
})
})
8 changes: 4 additions & 4 deletions integration-tests/debugger/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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('/'))
})
Expand All @@ -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,
Expand All @@ -97,7 +97,7 @@ function setup () {
})

afterEach(async function () {
proc.kill()
t.proc.kill()
await t.agent.stop()
})

Expand Down
8 changes: 6 additions & 2 deletions packages/dd-trace/src/debugger/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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})`)
})
Expand All @@ -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) {
Expand Down

0 comments on commit 297c5c5

Please sign in to comment.