Discussion:
Disabling callbacks and closing - block until callbacks finished
(too old to reply)
n***@4pmp.com
2014-10-13 08:14:09 UTC
Permalink
Raw Message
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 ***@freehaven.net with
unsubscribe libevent-users in the body.
n***@4pmp.com
2014-10-13 13:46:56 UTC
Permalink
Raw Message
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.
Post by n***@4pmp.com
----------
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
1. Disable all callbacks
2. Close the connection
3. Remove all pending tasks in the task queue for this connection
--------
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.
---------
Do the methods work like this, or is there a way to achieve this?
Many thanks in advance.
Nick.
***********************************************************************
unsubscribe libevent-users in the body.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Nick Giles
2014-10-14 20:07:39 UTC
Permalink
Raw Message
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!
Post by n***@4pmp.com
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
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.
Post by n***@4pmp.com
----------
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
1. Disable all callbacks
2. Close the connection
3. Remove all pending tasks in the task queue for this connection
--------
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.
---------
Do the methods work like this, or is there a way to achieve this?
Many thanks in advance.
Nick.
***********************************************************************
unsubscribe libevent-users in the body.
***********************************************************************
unsubscribe libevent-users in the body.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.

Loading...