Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Sandbox not terminating processes properly when running in Lambda (probably?) #594

Merged
merged 5 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

---

## [3.7.2] 2021-06-22

### Fixed

- Attempted to fix Sandbox not terminating processes properly when running in Lambda due to Lambda not having *nix `ps`

---

## [3.7.0 - 3.7.1] 2021-06-14

### Added
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@architect/sandbox",
"version": "3.7.1",
"version": "3.7.2-RC.1",
"description": "Architect dev server: run full Architect projects locally & offline",
"main": "src/index.js",
"scripts": {
Expand Down
30 changes: 27 additions & 3 deletions src/invoke-lambda/spawn.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
let { spawn } = require('child_process')
let { readdirSync } = require('fs')
let kill = require('tree-kill')
let { template } = require('../lib')
let { head } = template
Expand All @@ -7,6 +8,7 @@ const SIG = 'SIGINT'
module.exports = function spawnChild (params, callback) {
let { cwd, command, args, options, request, timeout, update } = params
let functionPath = options.cwd.replace(cwd, '').substr(1)
let isInLambda = process.env.AWS_LAMBDA_FUNCTION_NAME
let timedout = false
let headers = {
'content-type': 'text/html; charset=utf8;',
Expand Down Expand Up @@ -60,11 +62,33 @@ module.exports = function spawnChild (params, callback) {
update.error(`${functionPath} (pid ${pid}) caught hanging execution with an error, attempting to exit 1`)
code = 1
}
kill(pid, SIG, () => {
if (!isInLambda) {
kill(pid, SIG, () => {
murderInProgress = false
update.debug.status(`${functionPath} (pid ${pid}) successfully terminated`)
if (!closed) done(code)
})
}
else {
// tree-kill relies on *nix `ps`, which Lambda doesn't have – but it does have /proc
// Node process.kill() + Lambda Linux /proc/<pid>/task/<tid> is mysterious, so this may not be the best or proper approach
try {
let tasks = readdirSync(`/proc/${pid}/task`)
tasks.forEach(tid => {
try { process.kill(tid) }
catch (err) {
// Task may have ended naturally or been killed by killing child.pid, I guess we don't really know
update.debug.status(`${functionPath} (pid ${pid}) did not kill task (tid ${tid})`)
}
})
update.debug.status(`${functionPath} (pid ${pid}) (possibly maybe) successfully terminated inside Lambda`)
}
catch (err) {
update.debug.status(`${functionPath} (pid ${pid}) failed to terminate inside Lambda: ${err.message}`)
}
murderInProgress = false
update.debug.status(`${functionPath} (pid ${pid}) successfully terminated`)
if (!closed) done(code)
})
}
}
else {
update.debug.status(`${functionPath} (pid ${pid}) is not running (termination in progress: ${murderInProgress}; process closed: ${closed}`)
Expand Down
21 changes: 21 additions & 0 deletions test/integration/events-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,29 @@ test('invoke-lambda should respect timeout for sync functions and process should
})
})

test('invoke-lambda should respect timeout for sync functions and process should be killed (inside actual AWS Lambda)', t => {
let isLinux = process.platform === 'linux'
t.plan(1)
if (isLinux) {
process.env.AWS_LAMBDA_FUNCTION_NAME = 'yep'
let fileThatShouldNotBeWritten = join(tmp, 'foo-sync')
arc.events.publish({
name: 'event-timeout-sync',
payload: { path: fileThatShouldNotBeWritten }
},
function done (err) {
if (err) t.fail(err)
else setTimeout(() => {
t.notOk(existsSync(fileThatShouldNotBeWritten), 'file not created by event as event timed out and process was terminated appropriately')
}, 1250) // 1s is the configured timeout of test/mock/normal/src/events/event-timeout-sync so we pad it a bit and check after delay
})
}
else t.pass('Skipped because !Linux')
})

test('Sync events.end', t => {
t.plan(1)
delete process.env.AWS_LAMBDA_FUNCTION_NAME
events.end(function (err, result) {
if (err) t.fail(err)
else t.equal(result, 'Event bus successfully shut down', 'Events ended')
Expand Down