[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