Hi Gustavo,

On 21 September 2016 at 07:11, Carsten Haitzler <ras...@rasterman.com>
wrote:

> On Tue, 20 Sep 2016 12:17:50 -0300 Gustavo Sverzut Barbieri
> <barbi...@gmail.com> said:
>
> > On Tue, Sep 20, 2016 at 11:18 AM, Carsten Haitzler <ras...@rasterman.com
> >
> > wrote:
> > > On Tue, 20 Sep 2016 08:16:21 -0300 Gustavo Sverzut Barbieri
> > > <barbi...@gmail.com> said:
> > >
> > >> On Tue, Sep 20, 2016 at 5:51 AM, Jean-Philippe André <
> j...@videolan.org>
> > >> wrote:
> > >> > Hi Gustavo,
> > >> >
> > >> > On 14 September 2016 at 13:55, Gustavo Sverzut Barbieri
> > >> > <barbi...@gmail.com> wrote:
> > >> >
> > >> >> HEADS UP -- BINDINGS:
> > >> >>
> > >> >> If you wish to expose the new "cancellable" property, or cope with
> > >> >> naughty users or 3rd party deps that may call
> > >> >> pthread_setcancelstate(PTHREAD_CANCEL_ENABLE), please register
> your
> > >> >> cleanup functions with:
> > >> >>
> > >> >>    EINA_THREAD_CLEANUP_PUSH(cb, ctx); // will call cb() on
> > >> >> pthread_cancel()/ecore_thread_cancel()/eina_thread_cancel()
> > >> >>    call_user();
> > >> >>    EINA_THREAD_CLEANUP_POP(EINA_TRUE); // will call cb() on clean
> exit
> > >> >>
> > >> >> If different functions are desired:
> > >> >>
> > >> >>    EINA_THREAD_CLEANUP_PUSH(cb, ctx); // will call cb() on
> > >> >> pthread_cancel()/ecore_thread_cancel()/eina_thread_cancel()
> > >> >>    call_user();
> > >> >>    EINA_THREAD_CLEANUP_POP(EINA_FALSE); // will NOT call cb() on
> clean
> > >> >> exit
> > >> >>    another_cb(ctx);
> > >> >>
> > >> >> This is recommended if you expose ecore_thread to your users. If
> you
> > >> >> don't, then you do not need to do anything.
> > >> >
> > >> >
> > >> > pthread_cancel did (does?) not exist on Android, by design choice.
> bionic
> > >> > isn't posix in that regard.
> > >> > One way or another, I remember that using cancel is actually quite
> hard,
> > >> > because you need to be very careful about the cleanup.
> > >> >
> > >> > So I wonder if adding cancel like pthread here is the best choice?
> > >> > Note that I understand the need and don't have any good alternative
> :)
> > >>
> > >> Hi jpeg, thanks for letting me know.
> > >>
> > >> When we port to bionic we should then take one choice:
> > >>
> > >>  - let the thread hang there for a while, the new code I'm doing based
> > >> on this concept shouldn't break, it will just consume few more
> > >> resources for a while. (== #ifdef)
> > >
> > > i'd say do this all the time. cancellation otherwise is way too hard
> AND its
> > > non-portable. this is an api in eina that is non-portable and that
> means we
> > > have to try make it work on other platforms... if we keepit.
> >
> > by default the thread cancellation is disabled. You have to manually
> > enable that and live with the pain, If you wish.
> >
> > I've used new efl_net code to test that, simulate what our users that
> > loves thread would do... indeed it simplifies the code a lot to use a
> > thread and do procedural programming (see ecore_con.c, socks stuff).
> > The single detail to remember is to EINA_THREAD_CLEANUP_PUSH()/POP()
> > and take care to not return in the middle, since it wouldn't execute
> > the POP(EINA_TRUE) -- if you want to always cleanup.
> >
> > The portability part can be done, like vlc did. And that is said to
> > work on Windows, seems Android is the only one not doing it.
>

Funny you would mention VLC. The first implementation of vlc_thread_cancel
on Android relied on a signal which would hopefully interrupt the blocking
syscall in the thread to cancel. From my memories, this basically just did
the job. Not sure which drawbacks it had effectively, but at the time, Remi
had mentionned using futex instead.

The current code uses futex indeed. What I can't figure out is how this
interrupts blocking syscalls.
Honestly I don't understand how that code works now :( If someone can
enlighten me?

I also don't get how syscalls are interrupted on Windows.


not portable to android. the problem is we have an eina thread api that is
> not
> portable. this is not an internal implementation detail. it's a public api
> that
> is not portable. no. no non-portable api's anymore. DEFINITELY if they
> completely fail based on platform. if its "its a bit slower on another
> platform
> as that feature doesnt exist so we emulate" then fine.
>
> we used to be linux/unix only. over the years we've had to become
> portable. now
> we have to make portable the top priority.
>
> if it's not portable then don't support it. deal with it some other way.



>
> > >>  - use pthread_kill() with an unused signal and use a signal
> > >> handler... this can be a full wrapper like the one in videolan.org,
> or
> > >> just to generate EINTR like I proposed in my first discussion.
> > >
> > > this would be a far better workaround. :)
> >
> > doesn't work on Windows :-(
>
> then hang until timeout hit. use another thread in the pool. :)
>

This assumes that the syscalls will timeout.

Anyhow. pthread_cancel is something that in practice is very hard to deal
with properly. Variable and state cleanup is very tricky, and all functions
called within the cancellable thread must deal with cancellation. This
means you can't call any complex function. Cancellation is quite ideal for
a simple network read loop but total nightmare for complex stuff. Use with
caution.

-- 
Jean-Philippe André
------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to