#48 Better bufferevent socket shutdown code

Nick Mathewson

It would be good if we had a nice way to send shutdowns() and/or DisconnectEx calls to sockets on a bufferevent. Here are some ideas from http://archives.seul.org/libevent/users/Oct-2010/msg00068.html .

"Yes, what kind of API did you have in mind? The easiest would be
something to implement bufferevent_flush(BEV_FINSISHED) for sockets,
and define it to a) write as much as possible, and b) if all data is
written, shutdown() the socket.

But it seems that what people usually want instead is something that
will start writing, wait till all data is written, then shutdown() the
socket. I'm a little less thrilled with writing one of these, since
it's just a wrapper around other functionality, but it does seem to be
the kind of thing that people. It should probably be called something
like ... well, bufferevent_flush() is taken.
bufferevent_flush_and_shutdown(), maybe ? It should call the event
callback with a new event type, BEV_SENT_CLOSE, when it's finished.
It should also respect write timeouts.

Does anybody care strongly about this to try writing the code and tests?"


  • Kelly Brock
    Kelly Brock

    I'm going to write up a more detailed description of this on the e-mail list but there is a very definite reason that libevent is going to need to support a more integral variation of shutdown. Basically if you use threaded code you can't properly sequence the shutdown. This is most notable under IOCP with multiple background threads, but it will happen with other backends also as they increase processing thread counts.

    Basically with the seperation of EOF in the event callback and the write callback there is no way to "completely" synchronize the shutdown. Every once in a while even with enough locking to make it effectively single threaded, the EOF causes a race condition. There is just a enough libevent backend processing to cause such a race condition. I can't say that I don't have bugs but I've inserted full locking at all points and still can not solve the issue in an event based manner without an occasional failure.

    So, changes suggested are "remove" the EOF event and go back to the standard sockets method of "zero read == EOF" and then thread out correctly the final shutdown. I.e. with IOCP and eventually other backends you could have 3 read callbacks in flight at the same time. Order of processing can be completely random so detection of "EOF" is only part of the problem, the proper time to call bufferevent_free (or evbuffer_free for that matter) is near impossible to deal with.

    Currently I have to use a timer which sometimes causes data truncation if I insert performance issues in processing callbacks. It is a problem I can deal with, but it is not the optimal and prefered "perfectly clean" 100% reproducible solution for my unit tests.