Skip to content

Commit

Permalink
Merge pull request #160 from kalviumcommunity/fix/cleanup-multifile
Browse files Browse the repository at this point in the history 
Fix cleanup for multifile executions
  • Loading branch information
@anipnwr7777
anipnwr7777 authored Aug 21, 2024
2 parents e941f18 + a823fd1 commit 9083091
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 86 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ FROM docker.io/library/node:20.13.0-alpine

ENV PYTHONUNBUFFERED=1
RUN set -ex && \
apk add --no-cache gcc g++ musl-dev python3 openjdk17 ruby iptables ip6tables
apk add --no-cache gcc g++ musl-dev python3 openjdk17 ruby iptables ip6tables curl

RUN set -ex && \
apk add --no-cache chromium lsof
Expand Down
108 changes: 23 additions & 85 deletions services/code.service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -735,88 +735,6 @@ const _writeFilesToDisk = async (files, workingDir) => {
}
}

const _killProcessOnPort = async (port) => {
return new Promise((resolve, reject) => {
let isRejected = false
const lsof = spawn('lsof', ['-i', `:${port}`])

let stdout = ''
let stderr = ''

lsof.stdout.on('data', (data) => {
stdout += data.toString();
});

lsof.stderr.on('data', (data) => {
stderr += data.toString();
});

lsof.on('close', (code) => {
logger.info(stdout)
if (code === 1 && stderr.trim() === '') { // lsof returns 1 when the port is idle
logger.info(`port ${port} is free`)
resolve()
} else if (stderr) {
logger.error(stderr)
if (!isRejected) {
isRejected = true
reject()
}
}
else if (stdout) {
logger.info(`Port ${port} is occupied. Attempting to kill the process...`)

let pid
const lines = stdout.trim().split('\n') // For Unix-based systems, get the second part of the output line (PID)
if (lines.length > 1) {
const line = lines[1]
pid = line.trim().split(/\s+/)[1]
}

logger.info('printing pid ', pid)
if (!pid || isNaN(pid)) {
logger.info(`Invalid PID: ${pid}`)
if (!isRejected) {
isRejected = true
reject()
}
}

const kill = spawn('kill', ['-15', pid], {
detached: true,
stdio: 'ignore',
});
kill.on('exit', (exitCode) => {
logger.info(`kill command exited with code ${exitCode}`)
})
kill.on('close', (killCode) => {
if (killCode !== 0) {
logger.info(`kill command closed with code ${killCode}`)
if (!isRejected) {
isRejected = true
reject()
}
} else {
resolve()
}
})
}
})
})
}

const _preCleanUp = async (multifileType) => {
try {
if(multifileType === NODEJS_JUNIT) return;
await _killProcessOnPort(appConfig.multifile.jasminePort)
// TODO: add pre cleanup for puppeteer and jasmine server to prevent memory leak
} catch (err) {
// since there was an error in pre clean up which is mandatory for running test setup
// we kill the current process and in turn container exits and new one is spun up.
logger.info(`Error in pre cleanup: ${err.message}`, { stack: err?.stack })
process.exit(1)
}
}

const _checkIntegrity = async (non_editable_files) => {
for (const [filePath, expectedHash] of Object.entries(non_editable_files)) {
Expand All @@ -843,17 +761,36 @@ const parseResults = async (filePath, testCaseFormat) => {
}
}

const _postCleanUp = async (type, staticServerInstance = undefined, jasmineServer = undefined) => {
await _cleanUpDir(appConfig.multifile.workingDir, appConfig.multifile.submissionFileDownloadPath)
switch (type) {
case FRONTEND_STATIC_JASMINE:
if(staticServerInstance) {
staticServerInstance.close(() => {
logger.info('Exiting static server in post cleanup')
})
}
break
case FRONTEND_REACT_JASMINE:
if(jasmineServer) {
logger.info('Exiting react setup server in post cleanup')
process.kill(-jasmineServer.pid)
}
break
}
}

const _executeMultiFile = async (req, res, response) => {
logger.info(`serving ${req.type}`)
try {
await _preCleanUp(req.type)
const fileContent = await _getSubmissionDataFromGCS(req.url, appConfig.multifile.submissionFileDownloadPath)
await _writeFilesToDisk(fileContent, appConfig.multifile.workingDir)
} catch (err) {
logger.error(err)
throw (new Error('Error in running multifile submission, check service logs for the issue'))
}

let staticServerInstance, jasmineServer
try {
let jasmineResults
if(req?.non_editable_files) {
Expand All @@ -862,7 +799,7 @@ const _executeMultiFile = async (req, res, response) => {
}
switch (req.type) {
case FRONTEND_STATIC_JASMINE:
const staticServerInstance = await _startStaticServer(appConfig.multifile.staticServerPath)
staticServerInstance = await _startStaticServer(appConfig.multifile.staticServerPath)
jasmineResults = await _runTests()
if (staticServerInstance) {
staticServerInstance.close(() => {
Expand All @@ -875,7 +812,7 @@ const _executeMultiFile = async (req, res, response) => {
throw new Error(`No package.json found`)
}
await _installDependencies(appConfig.multifile.workingDir)
const jasmineServer = await _startJasmineServer()
jasmineServer = await _startJasmineServer()
jasmineResults = await _runTests()
process.kill(-jasmineServer.pid) // kill entire process group including child process and transitive child processes
break
Expand All @@ -890,6 +827,7 @@ const _executeMultiFile = async (req, res, response) => {
await _cleanUpDir(appConfig.multifile.workingDir, appConfig.multifile.submissionFileDownloadPath)
response.output = jasmineResults
} catch (err) {
await _postCleanUp(req.type, staticServerInstance, jasmineServer)
if (err.message === 'No package.json found' || err.message.includes('Browser was not found at')) {
throw err
} else {
Expand Down

0 comments on commit 9083091

Please sign in to comment.