Nick Mathewson [Tue, 13 Apr 2010 02:24:54 +0000 (22:24 -0400)]
Add evbuffer_copyout to copy data from an evbuffer without draining
The evbuffer_remove() function copies data from the front of an
evbuffer into an array of char, and removes the data from the buffer.
This function behaves the same, but does not remove the data. This
behavior can be handy for lots of protocols, where you want the
evbuffer to accumulate data until a complete record has arrived.
Lots of people have asked for a function more or less like this, and
though it isn't too hard to code one from evbuffer_peek(), it is
apparently annoying to do it in every app you write. The
evbuffer_peek() function is significantly faster, but it requires that
the user be able to handle data in separate extents.
This patch also reimplements evbufer_remove() as evbuffer_copyout()
followed by evbuffer_drain(). I am reasonably confident that this
won't be a performance hit: the memcpy() overhead should dominate the
cost of walking the list an extra time.
Nick Mathewson [Tue, 30 Mar 2010 20:47:37 +0000 (16:47 -0400)]
Rewrite evbuffer_expand and its users
The previous evbuffer_expand was not only incorrect; it was
inefficient too. On all questions of time vs memory tradeoffs, it
chose to burn time in order to avoid wasting memory. The new code
tries to be a little more balanced: it only resizes an existing chain
when doing so doesn't require too much copying, and when failing to do
so would waste a lot of the chain's space.
This patch also rewrites evbuffer_chain_insert to work properly with
last_with_datap, and adds a few convenience functions to buffer.c.
Nick Mathewson [Sat, 27 Mar 2010 04:09:25 +0000 (00:09 -0400)]
Make the no_iovecs case of write_atmost compile
Apparently nobody had tested it before on a system that had sendfile.
Why would you have sendfile and not writev? Perhaps you're trying to
test the no-iovecs code to make sure it still works.
Nick Mathewson [Sat, 27 Mar 2010 03:18:40 +0000 (23:18 -0400)]
Replace last_with_data with a slightly smarter version
To implement evbuffer_expand() properly, you need to be able to
replace the last chunk that has data, which means that we need to keep
track of the the next pointer pointing to the last_with_data chunk,
not the last_with_data chunk itself.
Nick Mathewson [Wed, 31 Mar 2010 16:29:26 +0000 (12:29 -0400)]
Fix critical bug in evbuffer_write when writev is not available
evbuffer_pullup() returns NULL if you try to pull up more bytes than
are there. But evbuffer_write_atmost would sometimes ask for more
bytes to be pulled up than it had, get a NULL, and fail.
Nick Mathewson [Fri, 26 Mar 2010 18:51:39 +0000 (14:51 -0400)]
Make evbuffer_prepend handle empty buffers better
If the first chunk of a buffer is empty, and we're told to prepend to
the buffer, we should be willing to use the entire first chunk.
Instead, we were dependent on the value of chunk->misalign.
Nick Mathewson [Fri, 26 Mar 2010 18:30:14 +0000 (14:30 -0400)]
Increase MIN_BUFFER_SIZE to 512 (1024 on 64-bit)
This constant decides the smallest (and typical) size of each evbuffer
chain. Since this number includes sizeof(evbuffer_chain) overhead,
the old value (256) was just too low: on 64-bit platforms, it would
spend nearly 20% of the allocations on overhead. The new values mean
that we'll be spending closer to 5% of evbuffer allocations on overhead.
It would be nice to get this number even lower if we can.
Nick Mathewson [Fri, 26 Mar 2010 17:46:29 +0000 (13:46 -0400)]
Remove a needless min_heap_shift_up_() call
Previously, every call to min_heap_shift_down_() would invoke
min_heap_shift_up_() at the end. This used to be necessary in the
first version of the minheap code, since min_heap_erase() would call
min_heap_shift_down_() unconditionally. But when patch 8b7a3b36763
from Marko Kreen fixed min_heap_erase() to be more sensible, we left
the weird behavior of min_heap_shift_down_() in place.
Fortunately, "cui" noticed this and reported it on Niels's blog.
Trond Norbye [Tue, 23 Mar 2010 17:27:10 +0000 (13:27 -0400)]
Never test for select() on windows
On 64-bit windows, configure actually _finds_ select when it tests for
it, and due to the ordering of the io implementations in event.c it is
chosen over the win32select implementation.
This modification skips the test for select on win32 (we don't want
that anyway, because Windows has its own), causing my windows box to
get the win32select implementation.
Nick Mathewson [Sun, 21 Mar 2010 17:28:48 +0000 (13:28 -0400)]
Detect and refuse reentrant event_base_loop() calls
Calling event_base_loop on a base from inside a callback invoked by
that same base, or from two threads at once, has long been a way to
get exceedingly hard-to-diagnose errors. This patch adds code to
detect such reentrant invocatinos, and exit quickly with a warning
that should explain what went wrong.
Nick Mathewson [Tue, 16 Mar 2010 17:37:15 +0000 (13:37 -0400)]
Make 'main/many_events' test 70 fds, not 64.
This is mainly intended to ensure that we don't get hung up on
the 64-handle limit that lots of O(n) Windows functions (but FWICT
not select) like to enforce.
Nick Mathewson [Sat, 13 Mar 2010 06:04:30 +0000 (01:04 -0500)]
Avoid an (untriggerable so far) crash bug in bufferevent_free()
We were saying
mm_free(bufev - bufev->be_ops->mem_offset);
when we should have said
mm_free(((char*)bufev) - bufev->be_ops->mem_offset);
In other words, if mem_offset had ever been nonzero, then instead of
backing up mem_offset bytes to find the thing we were supposed to free, we
would have backed up mem_offset*sizeof(struct bufferevent) bytes, and freed
something completely crazy.
Spotted thanks to a conversation with Jardel Weyrich
Nick Mathewson [Sat, 13 Mar 2010 05:53:54 +0000 (00:53 -0500)]
Free evdns_base->req_heads on evdns_base_free
It looks like when we moved from one big inflight-requests list to an
n-heads structure, we didn't make evdns_base_free() free the array of
heads. This patch should fix that.
Nick Mathewson [Fri, 12 Mar 2010 23:35:15 +0000 (18:35 -0500)]
Avoid a spurious close(-1) on Linux
On Linux, we use only one fd to do main-thread signaling (since we have
eventfd()), so we don't need to close th_notify_fd[1] as we would if we were
using a socketpair.
Nick Mathewson [Fri, 12 Mar 2010 19:37:54 +0000 (14:37 -0500)]
Give a better warning for bad automake versions.
If you tried to build with automake-1.6 or earlier, we would
previously spit out pages and pages of garbage output. Now, automake
should just say "Hey, I'm not new enough for this."
Nick Mathewson [Fri, 12 Mar 2010 19:16:30 +0000 (14:16 -0500)]
Switch to using AM conditionals in place of AC_LIBOBJ
AC_LIBOBJ is really only meant for defining missing library functions,
not conditional code compilation. Sticking our conditionally compiled
modules in SYS_SRC should make stuff easier to maintain.
Nick Mathewson [Thu, 11 Mar 2010 04:39:30 +0000 (23:39 -0500)]
Allow evbuffer_read() to split across more than 2 iovecs
Previously it would only accept 2 iovecs at most, because our
previous_to_last nonsense didn't let it take any more. This forced us
to do more reallocations in some cases when an extra small malloc
would have sufficed.
Nick Mathewson [Thu, 11 Mar 2010 04:24:14 +0000 (23:24 -0500)]
Use last_with_data in place of previous_to_last
This actually makes some of the code a lot simpler. The only
ones that actually used previous_to_last for anything were reserving
and committing space.
Nick Mathewson [Thu, 11 Mar 2010 03:16:14 +0000 (22:16 -0500)]
Revise evbuffer to add last_with_data
This is the first patch in a series to replace previous_to_last with
last_with_data. Currently, we can only use two partially empty chains
at the end of an evbuffer, so if we have one with 511 bytes free, and
another with 512 bytes free, and we try to do a 1024 byte read, we
can't just stick another chain on the end: we need to reallocate the
last one. That's stupid and inefficient.
Instead, this patch adds a last_with_data pointer to eventually
replace previous_to_last. Instead of pointing to the penultimated
chain (if any) as previous_to_last does, last_with_data points to the
last chain that has any data in it, if any. If all chains are empty,
last_with_data points to the first chain. If there are no chains,
last_with_data is NULL.
The next step is to start using last_with_data everywhere that we
currently use previous_to_last. When that's done, we can remove
previous_to_last and the code that maintains it.
Nick Mathewson [Wed, 10 Mar 2010 21:25:16 +0000 (16:25 -0500)]
Make evdns use the regular logging system by default
Once, for reasons that made sense at the time, we had evdns.c use its
own logging subsystem with two levels, "warn" and "debug". This leads
to problems, since setting a log handler for Libevent wouldn't actually
trap these messages, since they weren't on by default, and since some of
the warns should really be msgs.
This patch changes the default behavior of evdns.c to log to
event_(debugx,warnx,msgx) by default, and adds a new (internal-use-only)
log level of EVDNS_LOG_MSG. Programs that set a evdns logging
function will see no change. Programs that don't will now see evdns
warnings reported like other warnings.
Nick Mathewson [Tue, 2 Mar 2010 22:00:06 +0000 (17:00 -0500)]
Improve the speed of evbuffer_readln()
This makes some cases of bench_http about 5% faster.
Our internal evbuffer_strpbrk() function was overly general (it tried
to handle all character sets when we only used it for "\r\n"), and
not very efficient (it called memchr once for each character in the
buffer until it found a \r or a \n). It actually showed up in some
profiles for HTTP testing, since evbuffer_readln() calls it when doing
loose CRLF detection. This patch replaces it with a faster
implementation.
Nick Mathewson [Wed, 3 Mar 2010 17:15:15 +0000 (12:15 -0500)]
Remove signal_assign() and signal_new() macros.
These were introduced and deprecated in the same version (2.0.1-alpha),
presumably in two-stage process. Everybody sane should be using
evsignal_assign() and evsignal_new() instead.
It looks like I accidentally removed most of WIN32-Code/event-config.h
when I was bumping the version. Fortunately, this happened when I
bumped to 2.0.4-alpha-dev rather than when I bumped to 2.0.4-alpha. :)
This patch restores the deleted parts of WIN32-Code/event-config.h
Nick Mathewson [Sun, 28 Feb 2010 03:27:13 +0000 (22:27 -0500)]
Small cleanups on freebsd-connect-refused patch.
There should be no need to call be_socket_enable: that does an
event_add(). What we really want to do is event_active(), to make
sure that the writecb is executed.
Also, there was one "} if () {" that was missing an else.
I've noted that the return value for evutil_socket_connect() is
getting screwy, but since that isn't an exported function, we can fix
it whenever.
Nick Mathewson [Sat, 20 Feb 2010 23:44:35 +0000 (18:44 -0500)]
Provide consistent, tested semantics for bufferevent timeouts
The different bufferevent implementations had different behavior for
their timeouts. Some of them kept re-triggering the timeouts
indefinitely; some disabled the event immediately the first time a
timeout triggered. Some of them made the timeouts only count when
the bufferevent was actively trying to read or write; some did not.
The new behavior is modeled after old socket bufferevents, since
they were here first and their behavior is relatively sane.
Basically, each timeout disables the bufferevent's corresponding
read or write operation when it fires. Timeouts are stopped
whenever we suspend writing or reading, and reset whenever we
unsuspend writing or reading. Calling bufferevent_enable resets a
timeout, as does changing the timeout value.
Nick Mathewson [Tue, 23 Feb 2010 19:24:10 +0000 (14:24 -0500)]
Fix a bug in resetting timeouts on persistent events when IO triggers.
When we fixed persistent timeouts to make them reset themselves
based on the previous scheduled time rather than the current
time... we made them do so regardless of whether the event was
triggering because of a timeout or not!
This was of course bogus. When a _timeout_ triggers, we should
schedule the event for N seconds based on the last
_schedule_ time... but when IO triggers, we should reset the
timeout for N seconds after now.
Nick Mathewson [Mon, 22 Feb 2010 20:38:23 +0000 (15:38 -0500)]
Make bufferevent_free() clear all callbacks immediately.
This should end the family of bugs where we call bufferevent_free()
while a pending callback is holding a reference on the bufferevent,
and the callback tries to invoke the user callbacks before it releases
its own final reference.
This means that bufferevent_decref() is now a separate function from
bufferevent_free().
Nick Mathewson [Sat, 20 Feb 2010 17:55:59 +0000 (12:55 -0500)]
Suspend read/write on bufferevents during hostname lookup
When we're doing a lookup in preparation for doing a connect, we
might have an unconnected socket on hand, and mustn't actually do
any reading or writing with it.