Yes, this commit avoided the deadlock, because pthread_join() was not called. But, the program was messed up because of the call to the rogue eina_semaphore_destroy(), so it was not a fix, just something that prevented some symptoms of the problem to manifest in one particular way.
Eina does not handle the thread itself. It just performs cleanup, but does not join. If we shutdown too fast (i.e. without joining), cleanup may never happen. So in this case, joining seems mandatory. Jean On Tue, Sep 5, 2017 at 2:39 PM, <[email protected]> wrote: > Okay, but wasnt that commit fixing the deadlock you had in the join? > > Further more, after evas_thread_exited is 1 only eina deals with this > thread, not evas_thread_renderer.c so not waiting for the thread there > seems to be fine IMO, but keeping the code smaller is probebly better. > > Greetings > Marcel Hollerbach > > On Tue, Sep 05, 2017 at 02:25:50PM +0200, Jean Guyomarc'h wrote: > > Hi, > > > > I think this is just wrong. > > > > It evas_thread_exited is True, it does not mean that the thread is > actually > > finished. eina_thread_join() should be called **unconditionally** in this > > case. > > It is specified that if the thread is terminated, then pthread_join() > > (eina_thread's backend) will return immediately. > > > > BR > > > > > > Jean > > > > On Tue, Sep 5, 2017 at 2:16 PM, Marcel Hollerbach < > > [email protected]> wrote: > > > > > bu5hm4n pushed a commit to branch master. > > > > > > http://git.enlightenment.org/core/efl.git/commit/?id= > > > e41d46c635bdf769d4b93da609883347168c719a > > > > > > commit e41d46c635bdf769d4b93da609883347168c719a > > > Author: Marcel Hollerbach <[email protected]> > > > Date: Mon Sep 4 19:44:03 2017 +0200 > > > > > > evas_thread: only join the thread if the thread is still alive > > > > > > otherwise we might join a invalid thread id, that could lead to a > > > deadlock. Lets not do that. > > > > > > ref T5245 > > > > > > @fix > > > --- > > > src/lib/evas/common/evas_thread_render.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/lib/evas/common/evas_thread_render.c > > > b/src/lib/evas/common/evas_thread_render.c > > > index bbde81a657..62f0ba1577 100644 > > > --- a/src/lib/evas/common/evas_thread_render.c > > > +++ b/src/lib/evas/common/evas_thread_render.c > > > @@ -260,8 +260,8 @@ evas_thread_shutdown(void) > > > goto timeout_shutdown; > > > } > > > } > > > - > > > - eina_thread_join(evas_thread_worker); > > > + if (!evas_thread_exited) > > > + eina_thread_join(evas_thread_worker); > > > timeout_shutdown: > > > eina_lock_free(&evas_thread_queue_lock); > > > eina_condition_free(&evas_thread_queue_condition); > > > > > > -- > > > > > > > > > > > ------------------------------------------------------------ > ------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > _______________________________________________ > > enlightenment-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > ------------------------------------------------------------ > ------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
