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

[Romain Vimont]

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().