Nick Mathewson [Wed, 8 Sep 2010 17:22:55 +0000 (13:22 -0400)]
Minimize calls to base_notify implementation functions, thereby avoiding needless syscalls
The trick here is that if we already told the base to wake up, and it
hasn't woken up yet, we don't need to tell it to wake up again. This
should help lots with inherently multithreaded code like IOCP.
Only process up to MAX_DEFERRED deferred_cbs at a time.
If threads queue callbacks while event_process_deferred_callbacks is
running, the loop may spin long enough to significantly skew timers.
A unit test stressing this behavior is also in this commit.
- Increment reference count of bufferevents before initiating overlapped
operations to prevent the destructor from being called while operations
are pending. The only portable way of canceling overlapped ops is to
close the socket.
- Translate error codes to WSA* codes.
- Better handling of errors.
- Add an interface to add and del "virtual" events. Because IOCP
bufferevents don't register any events with the base, the event loop
has no way of knowing they exist. This causes the loop to terminate
prematurely. event_base_{add,del}_virtual increment/decrement base's
event count so the loop runs while there are any enabled IOCP
bufferevents.
- Prevent evbuffer_{add,prepend}_buffer from moving read-pinned chains.
- Fix evbuffer_drain to handle read-pinned chains better.
- Raise the limit on WSABUFs from two to MAX_WSABUFS for overlapped reads.
Nick Mathewson [Thu, 2 Sep 2010 15:10:50 +0000 (11:10 -0400)]
Use the _func() replacements for open, fstat, etc in evutil.c on win32
Remember that in a fit of ANSI C compliance, Microsoft decided to
screw portability by renaming basically all the functions in unistd.h to
get prefixed with an understore.
For some reason, mingw didn't seem to mind, but at least some people's
compilers did: see bug 3044490.
Nick Mathewson [Thu, 2 Sep 2010 16:06:58 +0000 (12:06 -0400)]
Declare signal handler function as "__cdecl" on Windows.
I swear, they must have half a dozen different calling conventions.
(goes to check)
Holy crud. They actually do. There's __cdecl, __stdcall, __fastcall,
"thiscall", "naked" and the obsolete "__pascal", "__fortran", and
"__syscall". And don't forget WINAPI and __far.
Anyways, this should fix 3044488 if I got it right.
Nick Mathewson [Thu, 2 Sep 2010 15:36:44 +0000 (11:36 -0400)]
Move evkeyvalq into a separate header for evhttp_parse_query users
The evhttp_parse_query API is a bit misdesigned; all the other
evkeyvalq stuff is abstract and lets you get away with having a header
stub, but evhttp_parse_query seems to require that you instantiate an
empty evkeyvalq of your own.
Nick Mathewson [Wed, 1 Sep 2010 20:36:30 +0000 (16:36 -0400)]
Close th_notify_fds and open a new pair on reinit
After a fork, you want subthreads to wake up the event_base in the
child process, not to have the child process and the main process
fight over who wakes up whom.
Related to a problem found by Nicholas Marriott while debugging 3048812.
Nick Mathewson [Mon, 30 Aug 2010 15:35:06 +0000 (11:35 -0400)]
Fix a bug in our win32 condition implementation
The do ... while loop in our wait code could spin while waiting
because the event object wasn't reset until there were no longer any
events waiting to be woken up. We also want to reset the event object
if the count of events to wake up reaches zero.
Nick Mathewson [Tue, 17 Aug 2010 17:26:03 +0000 (13:26 -0400)]
Correctly detect failure to delete bufferevent read-timeout event
Gilad Benjamini noted that we check the error code for deleting a
write-timeout event twice, and the read timeout not at all. This
shouldn't be a bit problem, since it's really hard for a delete to
fail on a timeout-only event, but it's worth fixing.
Nick Mathewson [Tue, 17 Aug 2010 17:18:18 +0000 (13:18 -0400)]
Use conditions instead of current_event_lock to fix a deadlock.
Avi Bab correctly noted as bug 3044479 the fact that any thread
blocking on current_event_lock will do so while holding
th_base_lock, making it impossible for the currently running event's
callback to call any other functions that require th_base_lock.
This patch switches the current_event_lock code to instead use a
condition variable that we wait on if we're trying to mess with
a currently-executing event, and that we signal when we're done
executing a callback if anybody is waiting on it.
Nick Mathewson [Tue, 17 Aug 2010 17:15:34 +0000 (13:15 -0400)]
Add a condition variable backend, with implementations for pthreads and win32
The interface from the user's POV is similar to the locking
implementation: either provide a structure full of function
pointers, or just call evthread_use_*_threads() and everything will
be okay.
The internal interface is meant to vaguely resemble pthread_cond_*,
which Windows people will better recognize as *ConditionVariable*.
Gilad Benjamini [Fri, 13 Aug 2010 21:08:59 +0000 (17:08 -0400)]
Clean up syntax on TAILQ_ENTRY() usage
Though the C standards allow it, it's apparently possible to get MSVC
upset by saying "struct { int field; } (declarator);" instead of
"struct {int field; } declarator;", so let's just not do that.
Nick Mathewson [Fri, 13 Aug 2010 15:41:37 +0000 (11:41 -0400)]
Make include/event2/event-config.h not included in source dist
As a generated file, it shouldn't get included in our source
distribution. Apparently there is an automake incant for this:
nobase_ even stacks with nodist_ .
Nick Mathewson [Fri, 13 Aug 2010 15:34:39 +0000 (11:34 -0400)]
Change include order in Makefile.nmake
If there is an event-config.h in include/event2 (either because we
screwed up packaging like in 2.0.6-rc or because we previously tried
building with mingw and we didn't make distclean in the middle), we
want MSVC to find the one one in WIN32-Code/include/event2 first.
Nick Mathewson [Mon, 9 Aug 2010 16:08:40 +0000 (12:08 -0400)]
Fix a nasty dangling-event bug when using rate-limiting groups
When we freed a bufferevent that was in a rate-limiting group and
blocked on IO, the process of freeing it caused it to get removed
from the group. But removing the bufferevent from the group made
its limits get removed, which could make it get un-suspended and in
turn cause its events to get re-added. Since we would then
immediately _free_ the events, this would result in dangling
pointers.
Nick Mathewson [Fri, 6 Aug 2010 20:36:23 +0000 (16:36 -0400)]
Fix unit tests with -DUSE_DEBUG enabled
If you were to enable USE_DEBUG and slog through all 700+ MB of
debugging output, you'd find that one of the unit tests failed,
since it tested the debug logging code, but the string it expected
and the string it logged differed by a tab vs 2 spaces.
Nick Mathewson [Fri, 6 Aug 2010 17:01:32 +0000 (13:01 -0400)]
Turn our socketpair() replacement into its own function
This patch splits the formerly windows-only case of evutil_socketpair()
into an (internal-use-only) function named evutil_ersatz_socketpair(), and
makes it build and work right on non-Windows hosts.
We need this for convenience to test sendfile on solaris, where socketpair
can't give you an AF_INET pair, and sendfile() won't work on AF_UNIX.
Nick Mathewson [Wed, 4 Aug 2010 19:52:32 +0000 (15:52 -0400)]
Fix an assertion bug in test-ratelim
If the rate limit was low enough, then the echo_conns wouldn't finish
inside the 300 msec we allowed for them to close. Instead, count the
number of connections we have, and keep waiting until they are all
closed.
Nick Mathewson [Wed, 4 Aug 2010 18:54:38 +0000 (14:54 -0400)]
Fix rate-limit calculation on openssl bufferevents.
When you're doing rate limiting on an openssl connection, you nearly
always want to limit the number of bytes sent and received over the
wire, not the number of bytes read or written over the secure
transport.
Nick Mathewson [Sat, 31 Jul 2010 21:10:04 +0000 (17:10 -0400)]
Build more cleanly with NetBSDs that dislike toupper(char)
To be fair, when char can be signed, if toupper doesn't take negative
characters, toupper(char) is a very bad idea. So let's just use the
nice safe EVUTIL_TOUPPER instead. (It explicitly only upcases ASCII,
but we only use it for identifiers that we know to be ASCII anyway).
Joachim Bauch [Fri, 30 Jul 2010 00:32:40 +0000 (20:32 -0400)]
Fix badly-behaved subtest of dns/bufferevent_connect_hostname
The bufferevent_connect_hostname test was specifying AF_INET, but the
gethostbyname test we were using to see what error to expect was using
PF_UNSPEC, leading to possible divergence of results.
Mike Smellie [Mon, 19 Jul 2010 03:31:19 +0000 (15:31 +1200)]
Change bufferevent_openssl::do_write so it doesn't call SSL_write with a 0 length buffer
I was running into a problem when using bufferevent_openssl with a
very simple echo server. My server simply bufferevent_read_buffer 'd
data into an evbuffer and then passed that evbuffer straight to
bufferevent_write_buffer.
The problem was every now and again the write would fail for no
apparent reason. I tracked it down to SSL_write being called with the
amount of data to send being 0.
This patch alters do_write in bufferevent_openssl so that it skips
io_vecs with 0 length.
Nick Mathewson [Mon, 19 Jul 2010 13:14:10 +0000 (15:14 +0200)]
Make test/test.sh call test-changelist
Eventually test-changelist should expand to try more cases, maybe
query the status of the actual changelist somehow, and integrate it
with the rest of the unit tests.
Mike Smellie [Mon, 19 Jul 2010 01:44:56 +0000 (13:44 +1200)]
Possible fix to 100% cpu usage with epoll and openssl
I'm running a fairly simple bit of test code using libevent2 with epoll and
openssl bufferevents and I've run into a 100% cpu usage problem.
Looking into it 100% usage was caused by epoll_wait constantly
returning write events on the openssl socket when it shouldn't really have
been looking for write events at all (N_ACTIVE_CALLBACKS() was returning 0
also).
Looking a bit deeper eventbuffer_openssl socket seems to be requesting
that the EV_WRITE event be removed when it should, but the event isn't
actually being removed from epoll.
Continuing to follow this I think I've found a bug in
event_changelist_del.
For evpoll event_del calls event_changelist_del which caches the change
which is then actioned later when evpoll_dispatch is called.
In event_changlist_del there is a check so that if the currently changed
action is an add then the cached action is changed to a no-op rather than a
delete (which makes sense). The problem arises if there are more than
two add or delete operations between calls to dispatch, in this case it's
possible that the delete is turned into a no-op when it shouldn't have
been.
For example starting with the event on, a delete followed by an add and
then another delete results in a no-op when it should have been a delete (I
added a fair bit of debug output that seems to confirm this behaviour).
I've applied a small change that checks the original old_event stored with
the change and only converts the delete to a no-op if the event isn't on in
old_event. This seems to have fixed my problem.
Fix wrong sie calculation of iovec buffers when exact=1
The old code had a bug where the 'exact' flag to 1 in
_evbuffer_read_setup_vecs would never actually make the iov_len field
of the last iovec get truncated. This patch fixes that.
Nick Mathewson [Tue, 13 Jul 2010 15:06:08 +0000 (11:06 -0400)]
Pass flags to fcntl(F_SETFL) and fcntl(F_SETFD) as int, not long
Everybody but Linux documents this as taking an int, and Linux is
very tolerant of getting an int instead. If it weren't, everybody
doing fcntl(fd,F_SETFL,O_NONBLOCK) would break, since the glibc
headers define O_NONBLOCK as an int literal.
Nick Mathewson [Thu, 8 Jul 2010 18:41:02 +0000 (14:41 -0400)]
Suppress a spurious EPERM warning in epoll.c
It's okay for us to get an EPERM when doing an EPOLL_DEL on an fd; it
just means that before we got a chance to the EPOLL_DEL, we closed the
fd and reopened a new non-socket that wound up having the same fd.
Nick Mathewson [Mon, 5 Jul 2010 18:39:39 +0000 (14:39 -0400)]
Don't race when calling event_active/event_add on a running signal event
There was previously no lock protecting the signal event's
ev_ncalls/ev_pncalls fields, which were accessed by all of
event_signal_closure, event_add_internal, event_del_internal, and
event_active_nolock. This patch fixes this race by using the
current_event_lock in the same way it's used to prevent
event_del_internal from touching an event that's currently running.