Re: [Libevent-users] Disabling callbacks and closing - block until callbacks finished

[Nick Giles <nick@xxxxxxxx>]

I think I've figured out what's going on - please correct me if I'm 
wrong (which is quite likely).   Perhaps by way of disclaimer I should 
mention that this is just what I could gather over a quick cup of tea 
this afternoon.

First of all, my previous post about locking has nothing to do with it, 
(I think).   The problem is that when bufferevent_free() is called, it 
does not block and wait for all currently registered callbacks for the 
connection to complete.   Instead it seems to do this:

1. Disable all callbacks via bufferevent_setcb(bufev, NULL, NULL, NULL, 
NULL);
2. Calls _bufferevent_cancel_all(bufev); which in the case of sockets 
doesn't appear to do anything [be_socket_ctrl()]
3. Decreases the buffervent's reference count by one 
[_bufferevent_decref_and_unlock()]
4. Returns immediately

The interesting part is the call to _bufferevent_decref_and_unlock() 
which for simplicity's sake looks a bit like this:

int
_bufferevent_decref_and_unlock(struct bufferevent *bufev)
{
    /* Declarations and other uninteresting stuff */

    if (--bufev_private->refcnt) {
        BEV_UNLOCK(bufev);
        return 0;
    }

    /* Closes the bufferevent and connection... */
}

When a bufferevent is initialised by bufferevent_socket_new(), 
bufev_private->refcnt is set to 1 [bufferevent_init_common()]. So when 
_bufferevent_decref_and_unlock() is called and there are no pending 
callbacks on the bufferevent, the refcnt will be decremented to 0, the 
condition will evaluate to false and the function will therefore 
continue to close the bufferevent and connection.

However, if there are pending callbacks on the bufferevent, then refcnt 
will be decrement to a positive value and the condition with evaluate to 
true, causing _bufferevent_decref_and_unlock() to return without closing 
the bufferevent.

This means that not only is the connection still open, but the callbacks 
currently registered in the event loop are unaffected and will be 
executed.   This means that there is potential for bufferevent activity 
_after_ the call to bufferevent_free().

(For completeness: once the last callback that was registered before our 
call to bufferevent_free() is executed, the condition in 
_bufferevent_decref_and_unlock() will evaluate to false and the 
bufferevent will finally be closed.)

Presuming my analysis is right, or at least right in the sense that 
there can still be bufferevent activity after a call to 
bufferevent_free(), then we can go back to my original problem: once 
bufferevent_free() has been called for a bufferevent, that bufferevent 
and associated connection data is moved to a scrapheap along with other 
closed connection structures.   If we realise we should free the 
scrapheap (i.e. the scrapheap is too big), then all the connection 
structures are freed, and as all this is happening on a different thread 
to the one on which the event loop is running, there is the potential 
that we could be freeing a connection for which there are still pending 
callbacks.

I need a way of telling if the connection has really been closed yet.

One possibility I see is that the disabling of callbacks sets the void 
pointer passed to each callback to null [bufferevent_setcb(bufev, NULL, 
NULL, NULL, NULL)].   If this will affect callbacks already scheduled 
such that the passed void pointer is NULL, then I could add a condition 
in the callbacks to immediately return if it's NULL.   Then when I'm 
freeing the scrapheap I can free everything except the bufferevents, 
knowing that if not already, they will eventually be freed when all the 
callbacks fire.

Another idea I had was some kind of callback when bufferevents are 
really actually closed, but I haven't looked yet to see if that exists.

I feel like I'm peeing in the dark a bit with this, so I'd very much 
appreciate any feedback either on the above analysis or on my proposed 
solutions.   I presume this must have been solved many times already so 
I'd love to hear from anyone with experience on this.

Thanks to everyone who's read this far and not fallen asleep!


On 13/10/14 15:46, nick@xxxxxxxx wrote:
> It occurred to me that it may be due to the fact that I am creating 
> bufferevents using the BEV_OPT_UNLOCK_CALLBACKS flag and not locking 
> the bufferevents when executing the tasks. Perhaps if the close task 
> does the following then the problem will disappear:
>
> 1. Lock the buffer event 
> (http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/bufferevent_8h.html#a56a61802037478591048b4967fa15599)
> 2. Disable all callbacks
> 3. Unlock the buffer event
> 4. Close the connection
> 5. Remove all pending tasks in the task queue for this connection
>
> Nick.
>
>
> Quoting nick@xxxxxxxx:
>
> > Situation:
> > ----------
> >
> > I am building a multithreaded socket server built with Libevent.   
> > The event loop is run in its own thread and each callback creates a 
> > task into a queue.   A pool of worker threads then processes the 
> > tasks, (of course ensuring that tasks for any given connection are 
> > processed in the correct order).
> >
> > When a close event arrives, a task is created to close the connection 
> > and free all related structures.   When this task is processed by a 
> > worker thread, it does the following things:
> >
> > 1. Disable all callbacks
> > 2. Close the connection
> > 3. Remove all pending tasks in the task queue for this connection
> >
> > Problem:
> > --------
> >
> > The problem is that after the connection is closed, there could still 
> > be events in the Libevent event loop either waiting to be processed 
> > in a callback or actually being processed by the event thread (i.e. 
> > callback running).   This could lead to tasks being created for the 
> > connection _after_ step 3 which would result in tasks in the task 
> > queue for a closed connection - which I want to avoid.
> >
> > Solution?
> > ---------
> >
> > I could use a mutex to lock the connection in the event callbacks, 
> > but I do not want to introduce blocking methods in the event loop 
> > thread.
> >
> > I could ensure that a connection is valid when I take a task.
> >
> > But ideally I would like to the bufferevent close method, or the 
> > callback disable methods to block until all curently running 
> > callbacks for that bufferevent have finished.   This way I could be 
> > sure that no tasks can be created after the connection is closed.
> >
> > Question:
> > ---------
> >
> > Do the methods work like this, or is there a way to achieve this?
> >
> > Many thanks in advance.
> >
> > Nick.
> >
> >
> > ***********************************************************************
> > To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
> > unsubscribe libevent-users    in the body.
>
>
>
> ***********************************************************************
> To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
> unsubscribe libevent-users    in the body.

***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.