| Message ID | 20230130221125.2299244-1-alex@linutronix.de |
|---|---|
| State | Accepted, archived |
| Commit | cd3e55606c427287f37585c5d7cde936471e52f4 |
| Headers | show |
| Series | oeqa/qemurunner: do not use Popen.poll() when terminating runqemu with a signal | expand |
Hi, On Mon, Jan 30, 2023 at 11:11:25PM +0100, Alexander Kanavin wrote: > This does not actually guarantee that the child runqemu process has completely exited: > poll() may return prematurely while the SIGTERM handler in runqemu is still running. > This thwarts the rest of the processing, and may terminate the handler before > it completes. > > Use Popen.communicate() instead: this is what python documentation recommends as well: > https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate Was I trying to solve the same problem in https://lists.openembedded.org/g/openembedded-core/message/176203 ? I think so. Cheers, -Mikko > Signed-off-by: Alexander Kanavin <alex@linutronix.de> > --- > meta/lib/oeqa/utils/qemurunner.py | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/meta/lib/oeqa/utils/qemurunner.py b/meta/lib/oeqa/utils/qemurunner.py > index b5fed6c9fe..8b893601d4 100644 > --- a/meta/lib/oeqa/utils/qemurunner.py > +++ b/meta/lib/oeqa/utils/qemurunner.py > @@ -543,10 +543,13 @@ class QemuRunner: > except OSError as e: > if e.errno != errno.ESRCH: > raise > - endtime = time.time() + self.runqemutime > - while self.runqemu.poll() is None and time.time() < endtime: > - time.sleep(1) > - if self.runqemu.poll() is None: > + try: > + outs, errs = self.runqemu.communicate(timeout = self.runqemutime) > + if outs: > + self.logger.info("Output from runqemu:\n%s", outs.decode("utf-8")) > + if errs: > + self.logger.info("Stderr from runqemu:\n%s", errs.decode("utf-8")) > + except TimeoutExpired: > self.logger.debug("Sending SIGKILL to runqemu") > os.killpg(os.getpgid(self.runqemu.pid), signal.SIGKILL) > if not self.runqemu.stdout.closed: > -- > 2.30.2 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#176502): https://lists.openembedded.org/g/openembedded-core/message/176502 > Mute This Topic: https://lists.openembedded.org/mt/96639889/7159507 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [mikko.rapeli@linaro.org] > -=-=-=-=-=-=-=-=-=-=-=- >
On Tue, 31 Jan 2023 at 08:38, Mikko Rapeli <mikko.rapeli@linaro.org> wrote: > On Mon, Jan 30, 2023 at 11:11:25PM +0100, Alexander Kanavin wrote: > > This does not actually guarantee that the child runqemu process has completely exited: > > poll() may return prematurely while the SIGTERM handler in runqemu is still running. > > This thwarts the rest of the processing, and may terminate the handler before > > it completes. > > > > Use Popen.communicate() instead: this is what python documentation recommends as well: > > https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate > > Was I trying to solve the same problem in > https://lists.openembedded.org/g/openembedded-core/message/176203 ? > > I think so. No, this is different. Your patch treats qemu process itself, mine fixes the problem with runqemu script. The whole qemurunner.py is a mess, for example it sends SIGTERM to both qemu and runqemu, where one or the other should be sufficient (runqemu will shutdown qemu if it is itself terminating, and will terminate if it detects that qemu finished). But I left that for another time. I only want a clean, reliable shutdown of runqemu. Alex
diff --git a/meta/lib/oeqa/utils/qemurunner.py b/meta/lib/oeqa/utils/qemurunner.py index b5fed6c9fe..8b893601d4 100644 --- a/meta/lib/oeqa/utils/qemurunner.py +++ b/meta/lib/oeqa/utils/qemurunner.py @@ -543,10 +543,13 @@ class QemuRunner: except OSError as e: if e.errno != errno.ESRCH: raise - endtime = time.time() + self.runqemutime - while self.runqemu.poll() is None and time.time() < endtime: - time.sleep(1) - if self.runqemu.poll() is None: + try: + outs, errs = self.runqemu.communicate(timeout = self.runqemutime) + if outs: + self.logger.info("Output from runqemu:\n%s", outs.decode("utf-8")) + if errs: + self.logger.info("Stderr from runqemu:\n%s", errs.decode("utf-8")) + except TimeoutExpired: self.logger.debug("Sending SIGKILL to runqemu") os.killpg(os.getpgid(self.runqemu.pid), signal.SIGKILL) if not self.runqemu.stdout.closed:
This does not actually guarantee that the child runqemu process has completely exited: poll() may return prematurely while the SIGTERM handler in runqemu is still running. This thwarts the rest of the processing, and may terminate the handler before it completes. Use Popen.communicate() instead: this is what python documentation recommends as well: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate Signed-off-by: Alexander Kanavin <alex@linutronix.de> --- meta/lib/oeqa/utils/qemurunner.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)