From: Armin Kuster <akuster808@gmail.com>
To: bitbake-devel@lists.openembedded.org
Subject: [1.44 04/18] knotty/uihelper: Switch from pids to tids for Task event management
Date: Sun, 22 Dec 2019 20:50:28 -0800 [thread overview]
Message-ID: <a109d034cf4fc059fd5a1e1d03246dac65522dd6.1577076251.git.akuster808@gmail.com> (raw)
In-Reply-To: <cover.1577076251.git.akuster808@gmail.com>
From: Richard Purdie <richard.purdie@linuxfoundation.org>
We've seen cases where a task can execute with a given pid, complete
and a new task can start using the same pid before the UI handler has
had time to adapt.
Traceback (most recent call last):
File "/home/pokybuild/yocto-worker/qemux86-alt/build/bitbake/lib/bb/ui/knotty.py", line 484, in main
helper.eventHandler(event)
File "/home/pokybuild/yocto-worker/qemux86-alt/build/bitbake/lib/bb/ui/uihelper.py", line 30, in eventHandler
del self.running_tasks[event.pid]
KeyError: 13490
This means using pids to match up events on the UI side is a bad
idea. Change the code to use task ids instead. There is a small
amount of fuzzy matching for the progress information since there
is no task information there and we don't want the overhead of a task
ID in every event, however since pid reuse is unlikely, we can live
with a progress bar not quite working properly in a corner case like
this.
[YOCTO #13667]
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit e427eafa1bb04008d12100ccc5c862122bba53e0)
---
lib/bb/build.py | 25 +++++++++++++------------
lib/bb/ui/knotty.py | 12 ++++++------
lib/bb/ui/uihelper.py | 39 ++++++++++++++++++++++++---------------
3 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/lib/bb/build.py b/lib/bb/build.py
index 30a2ba2..3d9cc10 100644
--- a/lib/bb/build.py
+++ b/lib/bb/build.py
@@ -57,8 +57,9 @@ builtins['os'] = os
class TaskBase(event.Event):
"""Base class for task events"""
- def __init__(self, t, logfile, d):
+ def __init__(self, t, fn, logfile, d):
self._task = t
+ self._fn = fn
self._package = d.getVar("PF")
self._mc = d.getVar("BB_CURRENT_MC")
self.taskfile = d.getVar("FILE")
@@ -81,8 +82,8 @@ class TaskBase(event.Event):
class TaskStarted(TaskBase):
"""Task execution started"""
- def __init__(self, t, logfile, taskflags, d):
- super(TaskStarted, self).__init__(t, logfile, d)
+ def __init__(self, t, fn, logfile, taskflags, d):
+ super(TaskStarted, self).__init__(t, fn, logfile, d)
self.taskflags = taskflags
class TaskSucceeded(TaskBase):
@@ -91,9 +92,9 @@ class TaskSucceeded(TaskBase):
class TaskFailed(TaskBase):
"""Task execution failed"""
- def __init__(self, task, logfile, metadata, errprinted = False):
+ def __init__(self, task, fn, logfile, metadata, errprinted = False):
self.errprinted = errprinted
- super(TaskFailed, self).__init__(task, logfile, metadata)
+ super(TaskFailed, self).__init__(task, fn, logfile, metadata)
class TaskFailedSilent(TaskBase):
"""Task execution failed (silently)"""
@@ -103,8 +104,8 @@ class TaskFailedSilent(TaskBase):
class TaskInvalid(TaskBase):
- def __init__(self, task, metadata):
- super(TaskInvalid, self).__init__(task, None, metadata)
+ def __init__(self, task, fn, metadata):
+ super(TaskInvalid, self).__init__(task, fn, None, metadata)
self._message = "No such task '%s'" % task
class TaskProgress(event.Event):
@@ -572,7 +573,7 @@ def _exec_task(fn, task, d, quieterr):
try:
try:
- event.fire(TaskStarted(task, logfn, flags, localdata), localdata)
+ event.fire(TaskStarted(task, fn, logfn, flags, localdata), localdata)
except (bb.BBHandledException, SystemExit):
return 1
@@ -583,15 +584,15 @@ def _exec_task(fn, task, d, quieterr):
for func in (postfuncs or '').split():
exec_func(func, localdata)
except bb.BBHandledException:
- event.fire(TaskFailed(task, logfn, localdata, True), localdata)
+ event.fire(TaskFailed(task, fn, logfn, localdata, True), localdata)
return 1
except Exception as exc:
if quieterr:
- event.fire(TaskFailedSilent(task, logfn, localdata), localdata)
+ event.fire(TaskFailedSilent(task, fn, logfn, localdata), localdata)
else:
errprinted = errchk.triggered
logger.error(str(exc))
- event.fire(TaskFailed(task, logfn, localdata, errprinted), localdata)
+ event.fire(TaskFailed(task, fn, logfn, localdata, errprinted), localdata)
return 1
finally:
sys.stdout.flush()
@@ -614,7 +615,7 @@ def _exec_task(fn, task, d, quieterr):
logger.debug(2, "Zero size logfn %s, removing", logfn)
bb.utils.remove(logfn)
bb.utils.remove(loglink)
- event.fire(TaskSucceeded(task, logfn, localdata), localdata)
+ event.fire(TaskSucceeded(task, fn, logfn, localdata), localdata)
if not localdata.getVarFlag(task, 'nostamp', False) and not localdata.getVarFlag(task, 'selfstamp', False):
make_stamp(task, localdata)
diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
index 35736ad..bd9911c 100644
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -255,19 +255,19 @@ class TerminalFilter(object):
start_time = activetasks[t].get("starttime", None)
if not pbar or pbar.bouncing != (progress < 0):
if progress < 0:
- pbar = BBProgress("0: %s (pid %s) " % (activetasks[t]["title"], t), 100, widgets=[progressbar.BouncingSlider(), ''], extrapos=2, resize_handler=self.sigwinch_handle)
+ pbar = BBProgress("0: %s (pid %s) " % (activetasks[t]["title"], activetasks[t]["pid"]), 100, widgets=[progressbar.BouncingSlider(), ''], extrapos=2, resize_handler=self.sigwinch_handle)
pbar.bouncing = True
else:
- pbar = BBProgress("0: %s (pid %s) " % (activetasks[t]["title"], t), 100, widgets=[progressbar.Percentage(), ' ', progressbar.Bar(), ''], extrapos=4, resize_handler=self.sigwinch_handle)
+ pbar = BBProgress("0: %s (pid %s) " % (activetasks[t]["title"], activetasks[t]["pid"]), 100, widgets=[progressbar.Percentage(), ' ', progressbar.Bar(), ''], extrapos=4, resize_handler=self.sigwinch_handle)
pbar.bouncing = False
activetasks[t]["progressbar"] = pbar
tasks.append((pbar, progress, rate, start_time))
else:
start_time = activetasks[t].get("starttime", None)
if start_time:
- tasks.append("%s - %s (pid %s)" % (activetasks[t]["title"], self.elapsed(currenttime - start_time), t))
+ tasks.append("%s - %s (pid %s)" % (activetasks[t]["title"], self.elapsed(currenttime - start_time), activetasks[t]["pid"]))
else:
- tasks.append("%s (pid %s)" % (activetasks[t]["title"], t))
+ tasks.append("%s (pid %s)" % (activetasks[t]["title"], activetasks[t]["pid"]))
if self.main.shutdown:
content = "Waiting for %s running tasks to finish:" % len(activetasks)
@@ -517,8 +517,8 @@ def main(server, eventHandler, params, tf = TerminalFilter):
continue
# Prefix task messages with recipe/task
- if event.taskpid in helper.running_tasks and event.levelno != format.PLAIN:
- taskinfo = helper.running_tasks[event.taskpid]
+ if event.taskpid in helper.pidmap and event.levelno != format.PLAIN:
+ taskinfo = helper.running_tasks[helper.pidmap[event.taskpid]]
event.msg = taskinfo['title'] + ': ' + event.msg
if hasattr(event, 'fn'):
event.msg = event.fn + ': ' + event.msg
diff --git a/lib/bb/ui/uihelper.py b/lib/bb/ui/uihelper.py
index c8dd7df..48d808a 100644
--- a/lib/bb/ui/uihelper.py
+++ b/lib/bb/ui/uihelper.py
@@ -15,39 +15,48 @@ class BBUIHelper:
# Running PIDs preserves the order tasks were executed in
self.running_pids = []
self.failed_tasks = []
+ self.pidmap = {}
self.tasknumber_current = 0
self.tasknumber_total = 0
def eventHandler(self, event):
+ # PIDs are a bad idea as they can be reused before we process all UI events.
+ # We maintain a 'fuzzy' match for TaskProgress since there is no other way to match
+ def removetid(pid, tid):
+ self.running_pids.remove(tid)
+ del self.running_tasks[tid]
+ if self.pidmap[pid] == tid:
+ del self.pidmap[pid]
+ self.needUpdate = True
+
if isinstance(event, bb.build.TaskStarted):
+ tid = event._fn + ":" + event._task
if event._mc != "default":
- self.running_tasks[event.pid] = { 'title' : "mc:%s:%s %s" % (event._mc, event._package, event._task), 'starttime' : time.time() }
+ self.running_tasks[tid] = { 'title' : "mc:%s:%s %s" % (event._mc, event._package, event._task), 'starttime' : time.time(), 'pid' : event.pid }
else:
- self.running_tasks[event.pid] = { 'title' : "%s %s" % (event._package, event._task), 'starttime' : time.time() }
- self.running_pids.append(event.pid)
+ self.running_tasks[tid] = { 'title' : "%s %s" % (event._package, event._task), 'starttime' : time.time(), 'pid' : event.pid }
+ self.running_pids.append(tid)
+ self.pidmap[event.pid] = tid
self.needUpdate = True
elif isinstance(event, bb.build.TaskSucceeded):
- del self.running_tasks[event.pid]
- self.running_pids.remove(event.pid)
- self.needUpdate = True
+ tid = event._fn + ":" + event._task
+ removetid(event.pid, tid)
elif isinstance(event, bb.build.TaskFailedSilent):
- del self.running_tasks[event.pid]
- self.running_pids.remove(event.pid)
+ tid = event._fn + ":" + event._task
+ removetid(event.pid, tid)
# Don't add to the failed tasks list since this is e.g. a setscene task failure
- self.needUpdate = True
elif isinstance(event, bb.build.TaskFailed):
- del self.running_tasks[event.pid]
- self.running_pids.remove(event.pid)
+ tid = event._fn + ":" + event._task
+ removetid(event.pid, tid)
self.failed_tasks.append( { 'title' : "%s %s" % (event._package, event._task)})
- self.needUpdate = True
elif isinstance(event, bb.runqueue.runQueueTaskStarted):
self.tasknumber_current = event.stats.completed + event.stats.active + event.stats.failed + 1
self.tasknumber_total = event.stats.total
self.needUpdate = True
elif isinstance(event, bb.build.TaskProgress):
- if event.pid > 0:
- self.running_tasks[event.pid]['progress'] = event.progress
- self.running_tasks[event.pid]['rate'] = event.rate
+ if event.pid > 0 and event.pid in self.pidmap:
+ self.running_tasks[self.pidmap[event.pid]]['progress'] = event.progress
+ self.running_tasks[self.pidmap[event.pid]]['rate'] = event.rate
self.needUpdate = True
else:
return False
--
2.7.4
next prev parent reply other threads:[~2019-12-23 4:50 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-23 4:50 [1.44 00/18] Patch review Armin Kuster
2019-12-23 4:50 ` [1.44 01/18] hashserv: Add support for equivalent hash reporting Armin Kuster
2019-12-23 4:50 ` [1.44 02/18] runqueue/siggen: Allow handling of equivalent hashes Armin Kuster
2019-12-23 8:25 ` Peter Kjellerstedt
2019-12-23 16:18 ` akuster808
2019-12-28 12:24 ` Richard Purdie
2019-12-23 4:50 ` [1.44 03/18] runqueue: Add extra debugging when locked sigs mismatches occur Armin Kuster
2019-12-23 4:50 ` Armin Kuster [this message]
2019-12-23 4:50 ` [1.44 05/18] siggen: Avoid taskhash mismatch errors for nostamp tasks when dependencies rehash Armin Kuster
2019-12-23 4:50 ` [1.44 06/18] siggen: Ensure new unihash propagates through the system Armin Kuster
2019-12-23 4:50 ` [1.44 07/18] runqueue: Batch scenequeue updates Armin Kuster
2019-12-23 4:50 ` [1.44 08/18] siggen: Fix performance issue in get_unihash Armin Kuster
2019-12-23 4:50 ` [1.44 09/18] bb.utils.fileslocked: don't leak files if yield throws Armin Kuster
2019-12-23 4:50 ` [1.44 10/18] runqueue: Rework process_possible_migrations() to improve performance Armin Kuster
2019-12-23 4:50 ` [1.44 11/18] runqueue: Fix task mismatch failures from incorrect logic Armin Kuster
2019-12-23 4:50 ` [1.44 12/18] siggen: Split get_tashhash for performance Armin Kuster
2019-12-23 4:50 ` [1.44 13/18] runqueue: Fix sstate task iteration performance Armin Kuster
2019-12-23 4:50 ` [1.44 14/18] runqueue: Optimise task migration code slightly Armin Kuster
2019-12-23 4:50 ` [1.44 15/18] runqueue: Optimise out pointless loop iteration Armin Kuster
2019-12-23 4:50 ` [1.44 16/18] runqueue: Optimise task filtering Armin Kuster
2019-12-23 4:50 ` [1.44 17/18] runqueue: Only call into the migrations function if migrations active Armin Kuster
2019-12-23 4:50 ` [1.44 18/18] lib/bb: Optimise out debug messages from cooker Armin Kuster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a109d034cf4fc059fd5a1e1d03246dac65522dd6.1577076251.git.akuster808@gmail.com \
--to=akuster808@gmail.com \
--cc=bitbake-devel@lists.openembedded.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.