[anaconda][master/f22-branch] Can't add thread AnaExecuteStorageThread (#1200613)
Jiri Konecny
jkonecny at redhat.com
Fri Mar 20 15:56:29 UTC 2015
> From: "Jiri Konecny" <jkonecny at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Friday, March 20, 2015 4:21:21 PM
> Subject: Re: [anaconda][master/f22-branch] Can't add thread AnaExecuteStorageThread (#1200613)
>
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Friday, March 20, 2015 8:59:17 AM
> Subject: Re: [anaconda][master/f22-branch] Can't add thread
> AnaExecuteStorageThread (#1200613)
>
> On Thu, 2015-03-19 at 17:16 +0100, Jiri Konecny wrote:
> > Before start of the new thread 'AnaExecuteStorageThread' wait for the old
> > thread
> > to finish.
> > Remove gtk_action_nowait to prevent blocking of main thread.
> > ---
> > pyanaconda/ui/gui/spokes/storage.py | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/pyanaconda/ui/gui/spokes/storage.py
> > b/pyanaconda/ui/gui/spokes/storage.py
> > index 8273dcb..bba16f9 100644
> > --- a/pyanaconda/ui/gui/spokes/storage.py
> > +++ b/pyanaconda/ui/gui/spokes/storage.py
> > @@ -308,11 +308,13 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> > # user may have set up before now.
> > self.storage.config.clearNonExistent = self.data.autopart.autopart
> >
> > - @gtk_action_nowait
> > def execute(self):
> > # Spawn storage execution as a separate thread so there's no big
> > delay
> > # going back from this spoke to the hub while StorageChecker.run
> > runs.
> > # Yes, this means there's a thread spawning another thread.
> > Sorry.
> > +
> > + # Wait for the existing thread to finish before starting the new
> > one.
> > + threadMgr.wait(name=constants.THREAD_EXECUTE_STORAGE)
> > threadMgr.add(AnacondaThread(name=constants.THREAD_EXECUTE_STORAGE,
> > target=self._doExecute))
> This is definitely a good check/thing to do and we should have it in the
> code. What's still a bit concerning to me though is that AFAICT it
> shouldn't be possible to enter the Storage spoke while the
> THREAD_EXECUTE_STORAGE thread is running. There are probably some weird
> races and this should prevent the crash caused by them, but we should
> probably at least have a chance to find out that we hit the race
> condition again. So, could you please add a line that would log a
> warning (or at least debug message) when we need to wait for the thread?
>
> Other than that this looks good to me.
>
> --
> Vratislav Podzimek
>
> ---------------------------------------------------------------------
>
> Thank you for the idea.
> I'll fix it locally by adding warning message.
>
Ok maybe I don't. I looked on the sources and I have an idea about upgrading
ThreadManager.wait method to return True or False if it wait's on the thread or not.
I think it will be better for the atomicity and usability.
This will be as separate patch for everyone to give feedback.
So for now I only push this without message and the print will be add later.
More information about the anaconda-patches
mailing list