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

[Alexandre Janniaux]

Hi,

On Wed, Sep 02, 2020 at 11:09:35AM +0200, Romain Vimont wrote:
> On Wed, Sep 02, 2020 at 10:34:28AM +0200, Romain Vimont wrote:
> > On Wed, Sep 02, 2020 at 08:13:15AM +0200, Steve Lhomme wrote:
> > > On 2020-09-01 18:13, Romain Vimont wrote:
> > > > +
> > > > +void
> > > > +vlc_executor_Submit(vlc_executor_t *executor, struct vlc_runnable *runnable)
> > > > +{
> > > > +    vlc_mutex_lock(&executor->lock);
> > > > +
> > > > +    assert(!executor->closing);
> > >
> > > It might be legitimate to try to submit a task even if the executor is
> > > closing. For example if a runnable is trying to submit another runnable. It
> > > should have to check with some higher power if it's allowed to do so or not
> > > (atomically). So at least you should just return here.
> >
> > In fact, just returning here will still cause problems: once submitted,
> > a runnable is either canceled (with vlc_executor_Cancel() returning
> > VLC_SUCCESS) or run (the run() callback is called).
> >
> > This allows the caller to know if the task is "finished" (so that it can
> > safely delete its resources).
> >
> > If a _Submit() is ignored, then the runnable will not be canceled and
> > will not be run, so resources will leak.
> >
> > I will think about it. I would like not to return a bool from _Submit()
> > just for that. I don't want to add a separate "deactivate" function for
> > closing before _Delete(). Maybe the threads should continue to dequeue
> > the pending tasks until the queue is empty in case a runnable queues
> > another one after "closing"?
>
> In fact, I think it is reasonable to assert that the executor is not
> closing. It is the caller responsibility to ensure that a runnable is
> not submitted after it called vlc_executor_Delete().

I agree, you don't want to Delete the executor while you still have
pending tasks.

Regards,
--
Alexandre Janniaux
Videolabs