Skip to content

Commit 6666173

Browse files
timvlaeralter-mage
andcommitted
fix: after killing child processes, kill parent process by sending a SIGTERM first
* fix: after killing child processes, kill parent process by sending a SIGTERM first * chore: ensure only living processes are killed --------- Co-authored-by: Siddhant Srivastava <sidsriv@amazon.com>
1 parent 7888fda commit 6666173

2 files changed

Lines changed: 13 additions & 7 deletions

File tree

src/main/java/com/aws/greengrass/util/platforms/unix/UnixExec.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,20 +123,21 @@ public void close() throws IOException {
123123
for (PidProcess pp : pidProcesses) {
124124
pp.waitFor(gracefulShutdownTimeout.getSeconds(), TimeUnit.SECONDS);
125125
}
126-
if (pidProcesses.stream().anyMatch(pidProcess -> {
126+
boolean isAnyProcessAlive = pidProcesses.stream().anyMatch(pidProcess -> {
127127
try {
128128
return pidProcess.isAlive();
129129
} catch (IOException ignored) {
130130
} catch (InterruptedException e) {
131131
Thread.currentThread().interrupt();
132132
}
133133
return false;
134-
})) {
134+
});
135+
if (isAnyProcessAlive) {
135136
logger.atWarn()
136137
.log("Command {} did not respond to interruption within timeout. Going to kill it now",
137138
this);
139+
platformInstance.killProcessAndChildren(p, true, pids, userDecorator);
138140
}
139-
platformInstance.killProcessAndChildren(p, true, pids, userDecorator);
140141
if (!p.waitFor(5, TimeUnit.SECONDS) && !isClosed.get()) {
141142
throw new IOException("Could not stop " + this);
142143
}

src/main/java/com/aws/greengrass/util/platforms/unix/UnixPlatform.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,13 @@ public UnixUserAttributes lookupCurrentUser() throws IOException {
275275
public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<Integer> additionalPids,
276276
UserDecorator decorator)
277277
throws IOException, InterruptedException {
278-
PidProcess pp = Processes.newPidProcess(process);
278+
PidProcess parentPidProcess = Processes.newPidProcess(process);
279279

280-
logger.atInfo().log("Killing child processes of pid {}, force is {}", pp.getPid(), force);
280+
logger.atInfo().log("Killing child processes of pid {}, force is {}", parentPidProcess.getPid(), force);
281281
Set<Integer> pids;
282282
try {
283283
pids = getChildPids(process);
284-
logger.atDebug().log("Found children of {}. {}", pp.getPid(), pids);
284+
logger.atDebug().log("Found children of {}. {}", parentPidProcess.getPid(), pids);
285285
if (additionalPids != null) {
286286
pids.addAll(additionalPids);
287287
}
@@ -293,6 +293,9 @@ public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<I
293293

294294
killProcess(force, decorator, pid);
295295
}
296+
297+
logger.atInfo().log("Killing parent process {}, force is {}", parentPidProcess.getPid(), force);
298+
killProcess(force, decorator, parentPidProcess.getPid());
296299
} finally {
297300
// calling process.destroy() here when force==false will cause the child process (component process) to be
298301
// terminated immediately. This prevents the component process from shutting down gracefully.
@@ -301,11 +304,13 @@ public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<I
301304
if (process.isAlive()) {
302305
// Kill parent process using privileged user since the parent process might be sudo which a
303306
// non-privileged user can't kill
304-
killProcess(true, getUserDecorator().withUser(getPrivilegedUser()), pp.getPid());
307+
killProcess(true, getUserDecorator().withUser(getPrivilegedUser()), parentPidProcess.getPid());
305308
}
306309
}
307310
}
308311

312+
// add parent pid so caller can validate if every process is properly terminated
313+
pids.add(parentPidProcess.getPid());
309314
return pids;
310315
}
311316

0 commit comments

Comments
 (0)