On Mon, Sep 19, 2016 at 8:08 PM, Carsten Haitzler <ras...@rasterman.com> wrote:
> On Mon, 19 Sep 2016 09:04:15 -0300 Gustavo Sverzut Barbieri
> <barbi...@gmail.com> said:
>
>> On Sun, Sep 18, 2016 at 2:03 PM, Carsten Haitzler <ras...@rasterman.com>
>> wrote:
>> > On Thu, 15 Sep 2016 08:00:46 -0300 Gustavo Sverzut Barbieri
>> > <barbi...@gmail.com> said:
>> >
>> >> isn't it better to check in ecore_thread_cancel() if the thread was
>> >> already created?
>> >
>> > that'd require keeping track of every thread we create, looking up in that
>> > list/table ever time etc. ... like eoid does. eina_thread stuff is are ally
>> > a tiny tiny tiny thin wrapper over posix (or windows or osx) threading -
>> > same with locks/semaphores etc. at this level such a complex check isn't
>> > really worth it.
>>
>> there is a single place inside ecore_thread_cancel() that calls this
>> function and it knows if t->self is 0 before calling.
>
> ummm... you're mixing ecore_thread and eina_thread. different things. :) this
> here is about eina_thread which has no t->self. t (thread) *IS* pthread_t. 
> it's
> opaque. well it hapens to be pthread_t but we could change it and alloc a
> struct in future as long as abi remains the same.
>
> so how do we check if t (Eina_Thread) has been created. basically we'd need to
>
> if (eina_list_find(all_eina_threads, t))
>   return pthread_cancel((pthread_t)t) == 0;
> return EINA_FALSE;
>
> that means a list walk every time to go find the handle. every
> eina_thread_create() has to:
>
> all_eina_threads = eina_list_append(all_eina_threads, t);
>
> We could use a hash. an array. point is we don't track all threads. we'd  have
> to start tracking them and to do a check would be a lookup like above.

we're talking about different things for different reasons :-D

I've added eina_thread_cancel() just to not expose pthread_cancel().
Most of users should never ever call that directly as they will do
mistakes with the threads. pthread_cancel() should return ESRCH if the
thread couldn't be found... not SEGV. And it's doing that for other
stuff, like pthread_join()... which we simply call without doing any
check on given Eina_Thread (thus would crash as well).

Users should use Ecore_Thread or something we may put on top (like
some Eo object). Ecore_Thread always provided ecore_thread_cancel(),
that only queues a request so users may ecore_thread_check() and stop
working.

With my change, ecore_thread_cancel() will call eina_thread_cancel(),
usually that's innocuous as thread cancellation is disabled. If the
user choose to enable thread cancellation, that is going to cancel the
thread preemptively at some well known points (man:pthreads(7)).

THUS, ecore_thread_cancel() does know if it create  the thread, if the
thread was already joined, etc. It can easily check and do not call
eina_thread_cancel().

Alternatively we may EINA_SAFETY_CHECK all entry point of
eina_thread.h, but I don't think we should do that given how low level
it is.



-- 
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (16) 99354-9890

------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to