Skip to content

Commit 160dafe

Browse files
nully0xbenalleng
authored andcommitted
chore: fix race condition issue (#53)
1 parent de416d1 commit 160dafe

File tree

2 files changed

+115
-128
lines changed

2 files changed

+115
-128
lines changed

src/lib/docker.ts

Lines changed: 107 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import Docker from 'dockerode'
44
export const docker = new Docker()
55

66
const MAX_SCRIPT_EXECUTION_TIME =
7-
Number(process.env.MAX_SCRIPT_EXECUTION_TIME) || 15000
7+
Number(process.env.MAX_SCRIPT_EXECUTION_TIME) || 15000
88

99
function buildImage(p, id, logStream, files) {
10-
return new Promise((resolve, reject) => {
10+
return new Promise((resolve, reject) => {
1111
docker.buildImage(
1212
{
1313
context: path.join(p),
@@ -18,7 +18,6 @@ function buildImage(p, id, logStream, files) {
1818
if (err) {
1919
reject(err)
2020
}
21-
console.log('buildImage1')
2221
stream.pipe(logStream)
2322
docker.modem.followProgress(stream, (err, res) => {
2423
if (err) {
@@ -28,108 +27,125 @@ function buildImage(p, id, logStream, files) {
2827
})
2928
}
3029
)
31-
})
30+
})
3231
}
3332

3433
function sanitizeContainerId(id) {
35-
return id.slice(0, 8)
34+
return id.slice(0, 8)
3635
}
3736

3837
// New cleanup function
39-
function cleanupContainerAndImage(container, imageId, send) {
40-
container.inspect((err, data) => {
41-
if (err) {
42-
console.error(`Failed to inspect container: ${err}`)
43-
} else {
44-
const containerState = data.State.Status
38+
async function cleanupContainerAndImage(container, imageId, send) {
39+
try {
40+
const data = await container.inspect()
4541

46-
if (containerState === 'running') {
47-
container.kill((err) => {
48-
if (err) {
49-
console.error(`Failed to kill container: ${err}`)
50-
} else {
51-
send({
52-
type: 'debug',
53-
payload: `[system] Container ${sanitizeContainerId(container.id)} killed.`,
54-
channel: 'runtime',
55-
})
56-
}
57-
58-
removeContainerAndImage(container, imageId, send)
42+
if (data.State.Status === 'running') {
43+
try {
44+
await container.kill()
45+
send({
46+
type: 'debug',
47+
payload: `[system] Container ${sanitizeContainerId(
48+
container.id
49+
)} killed.`,
50+
channel: 'runtime',
5951
})
60-
} else {
61-
removeContainerAndImage(container, imageId, send)
52+
} catch (err) {
53+
console.error(`Failed to kill container: ${err}`)
6254
}
6355
}
64-
})
56+
57+
await removeContainer(container, send)
58+
await removeImageIfNoContainers(imageId, send)
59+
} catch (err) {
60+
console.error(`Failed to inspect container: ${err}`)
61+
await removeContainer(container, send)
62+
await removeImageIfNoContainers(imageId, send)
63+
}
6564
}
6665

67-
function removeContainerAndImage(container, imageId, send) {
68-
container.remove((err) => {
69-
if (err) {
70-
console.error(`Failed to remove container: ${err}`)
71-
} else {
66+
async function removeImageIfNoContainers(imageId, send) {
67+
try {
68+
// List all containers using this image
69+
const containers = await docker.listContainers({
70+
all: true,
71+
filters: {
72+
ancestor: [imageId],
73+
},
74+
})
75+
76+
// Only remove the image if no containers are using it
77+
if (containers.length === 0) {
78+
await docker.getImage(imageId).remove({ force: true })
7279
send({
7380
type: 'debug',
74-
payload: `[system] Container ${sanitizeContainerId(container.id)} removed.`,
81+
payload: `[system] Image ${imageId} removed.`,
7582
channel: 'runtime',
7683
})
77-
78-
docker.getImage(imageId).remove((err) => {
79-
if (err) {
80-
console.error(`Failed to remove image: ${err}`)
81-
} else {
82-
send({
83-
type: 'debug',
84-
payload: `[system] Image ${imageId} removed.`,
85-
channel: 'runtime',
86-
})
87-
}
88-
})
84+
} else {
85+
console.log(
86+
`Image ${imageId} still has ${containers.length} containers, skipping removal`
87+
)
8988
}
90-
})
89+
} catch (err) {
90+
console.error(`Failed to remove image: ${err}`)
91+
}
92+
}
93+
94+
async function removeContainer(container, send) {
95+
try {
96+
await container.remove()
97+
send({
98+
type: 'debug',
99+
payload: `[system] Container ${sanitizeContainerId(
100+
container.id
101+
)} removed.`,
102+
channel: 'runtime',
103+
})
104+
} catch (err) {
105+
console.error(`Failed to remove container: ${err}`)
106+
}
91107
}
92108

93109
function runContainer(id, send, writeStream, context): Promise<boolean> {
94-
return new Promise(async (resolve, reject) => {
110+
return new Promise(async (resolve, reject) => {
111+
let isCleanedUp = false
112+
95113
send({
96114
type: 'debug',
97115
payload: '[system] Creating container...',
98116
channel: 'runtime',
99117
})
100118

101-
docker.createContainer({ Image: id, name: id, Tty: true },
119+
docker.createContainer(
120+
{ Image: id, name: id, Tty: true },
102121
(err, container) => {
103122
if (err) {
104123
return reject(err)
105124
}
106125

107-
console.log('container', container)
108-
109126
let isRunning = false
110-
111127
const containerId = sanitizeContainerId(container.id)
112128

113-
const job = context.jobs[context.socketId]
114-
job.container = container
115-
job.onKill = () => {
116-
isRunning = false
117-
cleanupContainerAndImage(container, id, send)
118-
writeStream.end()
119-
resolve(true)
129+
// Promise-based cleanup function
130+
const cleanup = async () => {
131+
if (!isCleanedUp) {
132+
isCleanedUp = true
133+
isRunning = false
134+
135+
try {
136+
writeStream.end()
137+
await cleanupContainerAndImage(container, id, send)
138+
resolve(true)
139+
} catch (error) {
140+
console.error('Cleanup failed:', error)
141+
reject(error)
142+
}
143+
}
120144
}
121145

122-
send({
123-
type: 'debug',
124-
payload: `[system] Container ${containerId} created.`,
125-
channel: 'runtime',
126-
})
127-
128-
send({
129-
type: 'debug',
130-
payload: `[system] Attaching WebSocket...`,
131-
channel: 'runtime',
132-
})
146+
const job = context.jobs[context.socketId]
147+
job.container = container
148+
job.onKill = cleanup
133149

134150
container.attach(
135151
{ stream: true, stdout: true, stderr: true },
@@ -138,68 +154,55 @@ function runContainer(id, send, writeStream, context): Promise<boolean> {
138154
return reject(err)
139155
}
140156

141-
send({
142-
type: 'debug',
143-
payload: `[system] WebSocket attached.`,
144-
channel: 'runtime',
145-
})
146-
147-
// Pipe container stdout to client.
148157
stream.pipe(writeStream)
149158

150-
send({
151-
type: 'debug',
152-
payload: `[system] Starting container ${containerId}...`,
153-
channel: 'runtime',
154-
})
155-
156159
// Listen for kill signal
157-
writeStream.onKill = () => {
158-
setTimeout(() => {
159-
isRunning = false
160+
writeStream.onKill = async () => {
161+
if (!isCleanedUp) {
160162
stream.unpipe(writeStream)
161-
writeStream.end()
162-
cleanupContainerAndImage(container, id, send)
163-
}, 1000)
163+
await cleanup()
164+
}
164165
}
165166

166-
setTimeout(() => {
167-
if (isRunning) {
168-
cleanupContainerAndImage(container, id, send)
167+
// Set timeout for max execution time
168+
const timeoutId = setTimeout(async () => {
169+
if (isRunning && !isCleanedUp) {
170+
stream.unpipe(writeStream)
171+
await cleanup()
169172
}
170173
}, MAX_SCRIPT_EXECUTION_TIME)
171174

172175
// Start the container
173176
container.start((err) => {
174177
if (err) {
178+
clearTimeout(timeoutId)
175179
return reject(err)
176180
}
177181

178182
isRunning = true
179183

180-
send({
181-
type: 'debug',
182-
payload: `[system] Container ${containerId} started.`,
183-
channel: 'runtime',
184-
})
184+
// Wait for container to stop
185+
container.wait(async (err, result) => {
186+
clearTimeout(timeoutId)
185187

186-
// Call the cleanup function when the container stops
187-
container.wait((err) => {
188188
if (err) {
189-
console.error(`Failed to wait for container to stop: ${err}`)
190-
} else {
191-
cleanupContainerAndImage(container, id, send)
189+
console.error(`Failed to wait for container to stop: ${err}`)
190+
}
191+
192+
if (!isCleanedUp) {
193+
stream.unpipe(writeStream)
194+
await cleanup()
192195
}
193196
})
194197
})
195198
}
196199
)
197200
}
198201
)
199-
})
202+
})
200203
}
201204

202205
export default {
203-
buildImage,
204-
runContainer,
206+
buildImage,
207+
runContainer,
205208
}

tsconfig.json

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,12 @@
99
"rootDir": "src",
1010
"baseUrl": "./src",
1111
"paths": {
12-
"lib/*": [
13-
"lib/*"
14-
],
15-
"middleware/*": [
16-
"middleware/*"
17-
],
18-
"models/*": [
19-
"models/*"
20-
],
21-
"routes/*": [
22-
"routes/*"
23-
],
24-
"types/*": [
25-
"types/*"
26-
],
27-
"config": [
28-
"config"
29-
]
12+
"lib/*": ["lib/*"],
13+
"middleware/*": ["middleware/*"],
14+
"models/*": ["models/*"],
15+
"routes/*": ["routes/*"],
16+
"types/*": ["types/*"],
17+
"config": ["config"]
3018
}
3119
// "strict": true,
3220
// "noImplicitAny": true,
@@ -40,10 +28,6 @@
4028
// "noImplicitReturns": true,
4129
// "noFallthroughCasesInSwitch": true
4230
},
43-
"include": [
44-
"src/**/*.ts"
45-
],
46-
"exclude": [
47-
"node_modules"
48-
]
31+
"include": ["src/**/*.ts"],
32+
"exclude": ["node_modules"]
4933
}

0 commit comments

Comments
 (0)