[vlc-devel] [RFC 1/2] executor: introduce new executor API

[Romain Vimont]

On Thu, Aug 27, 2020 at 04:10:03PM +0200, Alexandre Janniaux wrote:
> Hi,
> 
> On Thu, Aug 27, 2020 at 02:59:54PM +0200, Romain Vimont wrote:
> > On Wed, Aug 26, 2020 at 11:27:59PM +0300, Rémi Denis-Courmont wrote:
> > > Le keskiviikkona 26. elokuuta 2020, 23.11.08 EEST Romain Vimont a écrit :
> > > > > > + *      int ret = vlc_executor_Submit(executor, &runnable, str, NULL,
> > > > > > 0);
> > > > > > + *      assert(ret == VLC_SUCCESS);
> > > > > > + *  }
> > > > > > + * \endcode
> > > > > >
> > > > > > +VLC_API int
> > > > > > +vlc_executor_Submit(vlc_executor_t *executor,
> > > > > > +                    const struct vlc_runnable *runnable, void
> > > > > > *userdata,
> > > > > > +                    void *id, vlc_tick_t timeout);
> > > > >
> > > > > Type name for works and workers really ought to be in the same semantic
> > > > > field IMO.
> > > >
> > > > (the naming was inpired from ExecutorService in Java, but Executor is
> > > > quite "standard" in other languages too.)
> > > >
> > > > > It is unlikely that callers will (or even should have to) handle errors
> > > > > well at this point.
> > > >
> > > > The error handling is simple here:
> > > >  - VLC_SUCCESS: the runnable is submitted
> > > >  - other: there was an error, the runnable will not be executed
> > >
> > > Error handling is almost always "simple", meaning binary, by that definition.
> > >
> > > The point is that it is an inconvenient (and potentially impossible) state for
> > > the caller to handle errors. We don't normally allow thread functions to fail
> > > except at allocation/creation time (or not even).
> >
> > Since _Submit() allocates and may create a thread, it seems logical to
> > me to report an error.
> >
> > What would you suggest? I think that trying to not create/allocate here
> > will not lead to a better solution.
> 
> The thread could be allocated with the vlc_executor then?

vlc_executor_Submit() does several actions which may fail:
 - allocates internally a vlc_executor_task (there is one per call to
   Submit()) to store:
     - the parameters provided by the user
     - the task state while running
 - spawn a thread if necessary (when there are no tasks for a "long"
   time, the thread terminate)
 - now it also spawns the interrupt thread if is not created yet and if
   a timeout is requested (cf end of [1])

[1] https://mailman.videolan.org/pipermail/vlc-devel/2020-August/136703.html

> 
> >
> > > And thus this also breaks the principle of least surprise. If I've allocated a
> > > worker and a work succesfully, why should it fail to queue... Of course, if
> > > you look at the implementation, there are reasons. But that's not externally
> > > obvious...
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel