From: "Brian C. Lane" bcl@redhat.com
I have tested this against 18.37.11 and it works in that it will raise (and display) an exception from within a thread. It depends on something calling .wait() though, so it won't catch exceptions in threads that are fire-and-forget.
Brian C. Lane (2): Add error reporting to threadMgr Use threadMgr.wait to check threads
pyanaconda/packaging/__init__.py | 8 ++------ pyanaconda/packaging/livepayload.py | 4 +--- pyanaconda/threads.py | 29 +++++++++++++++++++++++++++++ pyanaconda/ui/gui/spokes/custom.py | 4 +--- pyanaconda/ui/gui/spokes/network.py | 4 +--- pyanaconda/ui/gui/spokes/software.py | 12 +++--------- pyanaconda/ui/gui/spokes/source.py | 8 ++------ pyanaconda/ui/gui/spokes/storage.py | 4 +--- pyanaconda/ui/tui/spokes/storage.py | 12 ++++-------- 9 files changed, 44 insertions(+), 41 deletions(-)
From: "Brian C. Lane" bcl@redhat.com
We need to be able to tell when a thread has quit because of an error. This adds the ability to save thread traceback information in threadMgr and a new method, .wait() that can be used instead of join. It will raise an error if the thread quits because of an Exception. --- pyanaconda/threads.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index 55fe141..6eca96b 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -35,6 +35,7 @@ class ThreadManager(object): """ def __init__(self): self._objs = {} + self._errors = {}
def __call__(self): return self @@ -47,6 +48,7 @@ class ThreadManager(object): raise KeyError
self._objs[obj.name] = obj + self._errors[obj.name] = None obj.start()
def remove(self, name): @@ -66,6 +68,32 @@ class ThreadManager(object): """ return self._objs.get(name)
+ def wait(self, name): + """Wait for the thread to exit and if the thread exited with an error + re-raise it here. + """ + if self.exists(name): + self.get(name).join() + if self._errors.get(name) is not None: + raise self._errors[name][0], self._errors[name][1], self._errors[name][2] + + def set_error(self, name, *exc_info): + """Set the error data for a thread + + The exception data is expected to be the tuple from sys.exc_info() + """ + self._errors[name] = exc_info + + def get_error(self, name): + """Get the error data for a thread using its name + """ + return self._errors.get(name) + + def any_errors(self): + """Return True of there have been any errors in any threads + """ + return any(self._errors.values()) + class AnacondaThread(threading.Thread): """A threading.Thread subclass that exists only for a couple purposes:
@@ -93,6 +121,7 @@ class AnacondaThread(threading.Thread): raise except: sys.excepthook(*sys.exc_info()) + threadMgr.set_error(self.name, *sys.exc_info()) finally: threadMgr.remove(self.name) log.info("Thread Done: %s (%s)" % (self.name, self.ident))
On Thu, 2013-01-24 at 15:21 -0800, Brian C. Lane wrote:
From: "Brian C. Lane" bcl@redhat.com
We need to be able to tell when a thread has quit because of an error. This adds the ability to save thread traceback information in threadMgr and a new method, .wait() that can be used instead of join. It will raise an error if the thread quits because of an Exception.
pyanaconda/threads.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/pyanaconda/threads.py b/pyanaconda/threads.py index 55fe141..6eca96b 100644 --- a/pyanaconda/threads.py +++ b/pyanaconda/threads.py @@ -35,6 +35,7 @@ class ThreadManager(object): """ def __init__(self): self._objs = {}
self._errors = {}def __call__(self): return self
@@ -47,6 +48,7 @@ class ThreadManager(object): raise KeyError
self._objs[obj.name] = obj
self._errors[obj.name] = None obj.start()def remove(self, name):
@@ -66,6 +68,32 @@ class ThreadManager(object): """ return self._objs.get(name)
- def wait(self, name):
"""Wait for the thread to exit and if the thread exited with an errorre-raise it here."""if self.exists(name):self.get(name).join()if self._errors.get(name) is not None:raise self._errors[name][0], self._errors[name][1], self._errors[name][2]- def set_error(self, name, *exc_info):
"""Set the error data for a threadThe exception data is expected to be the tuple from sys.exc_info()"""self._errors[name] = exc_info- def get_error(self, name):
"""Get the error data for a thread using its name"""return self._errors.get(name)- def any_errors(self):
"""Return True of there have been any errors in any threads"""return any(self._errors.values())class AnacondaThread(threading.Thread): """A threading.Thread subclass that exists only for a couple purposes:
@@ -93,6 +121,7 @@ class AnacondaThread(threading.Thread): raise except: sys.excepthook(*sys.exc_info())
threadMgr.set_error(self.name, *sys.exc_info())
I believe the last two lines should be swapped. It is not the case now, but sys.excepthook() invocation may end with sys.exit() or something like that resulting in no info added to threadMgr.
On Mon, Jan 28, 2013 at 11:02:19AM +0100, Vratislav Podzimek wrote:
On Thu, 2013-01-24 at 15:21 -0800, Brian C. Lane wrote:
From: "Brian C. Lane" bcl@redhat.com @@ -93,6 +121,7 @@ class AnacondaThread(threading.Thread): raise except: sys.excepthook(*sys.exc_info())
threadMgr.set_error(self.name, *sys.exc_info())I believe the last two lines should be swapped. It is not the case now, but sys.excepthook() invocation may end with sys.exit() or something like that resulting in no info added to threadMgr.
Thanks, done!
From: "Brian C. Lane" bcl@redhat.com
This will raise an exception if the thread quit because of an error. --- pyanaconda/packaging/__init__.py | 8 ++------ pyanaconda/packaging/livepayload.py | 4 +--- pyanaconda/ui/gui/spokes/custom.py | 4 +--- pyanaconda/ui/gui/spokes/network.py | 4 +--- pyanaconda/ui/gui/spokes/software.py | 12 +++--------- pyanaconda/ui/gui/spokes/source.py | 8 ++------ pyanaconda/ui/gui/spokes/storage.py | 4 +--- pyanaconda/ui/tui/spokes/storage.py | 12 ++++-------- 8 files changed, 15 insertions(+), 41 deletions(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index 35bde98..da4b301 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -663,15 +663,11 @@ class PackagePayload(Payload): def payloadInitialize(storage, ksdata, payload): from pyanaconda.threads import threadMgr
- storageThread = threadMgr.get("AnaStorageThread") - if storageThread: - storageThread.join() + threadMgr.wait("AnaStorageThread")
# FIXME: condition for cases where we don't want network # (set and use payload.needsNetwork ?) - networkThread = threadMgr.get("AnaWaitForConnectingNMThread") - if networkThread: - networkThread.join() + threadMgr.wait("AnaWaitForConnectingNMThread")
payload.setup(storage)
diff --git a/pyanaconda/packaging/livepayload.py b/pyanaconda/packaging/livepayload.py index ecade1a..ac55bbe 100644 --- a/pyanaconda/packaging/livepayload.py +++ b/pyanaconda/packaging/livepayload.py @@ -124,9 +124,7 @@ class LiveImagePayload(ImagePayload): # Wait for progress thread to finish with self.pct_lock: self.pct = 100 - progressThread = threadMgr.get("AnaLiveProgressThread") - if progressThread: - progressThread.join() + threadMgr.wait("AnaLiveProgressThread")
def postInstall(self): """ Perform post-installation tasks. """ diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index d14f6ba..94faef0 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -640,9 +640,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): # Make sure the storage spoke execute method has finished before we # copy the storage instance. for thread_name in ["AnaExecuteStorageThread", "AnaStorageThread"]: - t = threadMgr.get(thread_name) - if t: - t.join() + threadMgr.wait(thread_name)
self.passphrase = self.data.autopart.passphrase self._reset_storage() diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py index f89be9f..622e724 100644 --- a/pyanaconda/ui/gui/spokes/network.py +++ b/pyanaconda/ui/gui/spokes/network.py @@ -1120,9 +1120,7 @@ class NetworkStandaloneSpoke(StandaloneSpoke): from pyanaconda.packaging import payloadInitialize from pyanaconda.threads import threadMgr, AnacondaThread
- payloadThread = threadMgr.get("AnaPayloadThread") - if payloadThread: - payloadThread.join() + threadMgr.wait("AnaPayloadThread")
threadMgr.add(AnacondaThread(name="AnaPayloadThread", target=payloadInitialize, args=(self.storage, self.data, self.payload)))
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py index cbaf8f0..8598303 100644 --- a/pyanaconda/ui/gui/spokes/software.py +++ b/pyanaconda/ui/gui/spokes/software.py @@ -178,9 +178,7 @@ class SoftwareSelectionSpoke(NormalSpoke): def _initialize(self): communication.send_message(self.__class__.__name__, _("Downloading package metadata..."))
- payloadThread = threadMgr.get("AnaPayloadThread") - if payloadThread: - payloadThread.join() + threadMgr.wait("AnaPayloadThread")
communication.send_message(self.__class__.__name__, _("Downloading group metadata..."))
@@ -189,9 +187,7 @@ class SoftwareSelectionSpoke(NormalSpoke): if flags.automatedInstall and packagesSeen: # We don't want to do a full refresh, just # join the metadata thread - mdGatherThread = threadMgr.get("AnaPayloadMDThread") - if mdGatherThread: - mdGatherThread.join() + threadMgr.wait("AnaPayloadMDThread") else: if not self._first_refresh(): return @@ -222,9 +218,7 @@ class SoftwareSelectionSpoke(NormalSpoke): def refresh(self): NormalSpoke.refresh(self)
- mdGatherThread = threadMgr.get("AnaPayloadMDThread") - if mdGatherThread: - mdGatherThread.join() + threadMgr.wait("AnaPayloadMDThread")
self._environmentStore = self.builder.get_object("environmentStore") self._environmentStore.clear() diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py index 7faf0bc..cbd2514 100644 --- a/pyanaconda/ui/gui/spokes/source.py +++ b/pyanaconda/ui/gui/spokes/source.py @@ -678,15 +678,11 @@ class SourceSpoke(NormalSpoke):
communication.send_message(self.__class__.__name__, _("Probing storage..."))
- storageThread = threadMgr.get("AnaStorageThread") - if storageThread: - storageThread.join() + threadMgr.wait("AnaStorageThread")
communication.send_message(self.__class__.__name__, _(METADATA_DOWNLOAD_MESSAGE))
- payloadThread = threadMgr.get("AnaPayloadThread") - if payloadThread: - payloadThread.join() + threadMgr.wait("AnaPayloadThread")
added = False cdrom = None diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index 9cbc6d8..9aa510f 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -513,9 +513,7 @@ class StorageSpoke(NormalSpoke, StorageChecker): def _initialize(self): communication.send_message(self.__class__.__name__, _("Probing storage..."))
- storageThread = threadMgr.get("AnaStorageThread") - if storageThread: - storageThread.join() + threadMgr.wait("AnaStorageThread")
self.disks = getDisks(self.storage.devicetree)
diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index b5d483f..5314438 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -183,11 +183,9 @@ class StorageSpoke(NormalTUISpoke): NormalTUISpoke.refresh(self, args)
# Join the initialization thread to block on it - initThread = threadMgr.get("AnaStorageWatcher") - if initThread: - # This print is foul. Need a better message display - print(_("Probing storage...")) - initThread.join() + # This print is foul. Need a better message display + print(_("Probing storage...")) + threadMgr.wait("AnaStorageWatcher")
# synchronize our local data store with the global ksdata # Commment out because there is no way to select a disk right @@ -301,9 +299,7 @@ class StorageSpoke(NormalTUISpoke): # Secondary initialize so wait for the storage thread # to complete before populating our disk list
- storageThread = threadMgr.get("AnaStorageThread") - if storageThread: - storageThread.join() + threadMgr.wait("AnaStorageThread")
self.disks = sorted(getDisks(self.storage.devicetree), key=lambda d: d.name)
On Thu, 2013-01-24 at 15:21 -0800, Brian C. Lane wrote:
From: "Brian C. Lane" bcl@redhat.com
I have tested this against 18.37.11 and it works in that it will raise (and display) an exception from within a thread. It depends on something calling .wait() though, so it won't catch exceptions in threads that are fire-and-forget.
Brian C. Lane (2): Add error reporting to threadMgr Use threadMgr.wait to check threads
pyanaconda/packaging/__init__.py | 8 ++------ pyanaconda/packaging/livepayload.py | 4 +--- pyanaconda/threads.py | 29 +++++++++++++++++++++++++++++ pyanaconda/ui/gui/spokes/custom.py | 4 +--- pyanaconda/ui/gui/spokes/network.py | 4 +--- pyanaconda/ui/gui/spokes/software.py | 12 +++--------- pyanaconda/ui/gui/spokes/source.py | 8 ++------ pyanaconda/ui/gui/spokes/storage.py | 4 +--- pyanaconda/ui/tui/spokes/storage.py | 12 ++++-------- 9 files changed, 44 insertions(+), 41 deletions(-)
Apart from the one comment this looks good to me. Nice idea!
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
Vratislav Podzimek