This patch mark testcases that only fail under travis-ci/appveyor with
TT_RETRIABLE, since otherwise there is too much noise, other issues
(like failures under vagrant boxes) would be investigated separatelly.
linux (from travis-ci only):
- http/cancel_by_host_no_ns
- http/cancel_by_host_inactive_server
- http/cancel_by_host_ns_timeout
- http/cancel_by_host_ns_timeout_inactive_server
- thread/conditions_simple
- util/monotonic_prc_precise
- util/usleep
- main/del_wait
vagrant/ubuntu box (this is the only exception):
- thread/no_events
win32 (from appveyor only):
- main/active_later
- main/persistent_active_timeout
And we should use TT_RETRIABLE over TT_OFF_BY_DEFAULT/TT_SKIP when it
make sense.
But there is still "test-ratelim__group_lim" left.
We have some tests that has false-positive due to real/CPU time bound,
but they are pretty generic and we do not want to skip them by default.
TT_RETRIABLE is the flag that will indicate tinytest to retry the test
in case of failure, use it to avoid next possible false-positives:
- real time-related
- CPU time-related
Since I guess it is better to see/grepping RETRYING messages over
ignoring completely failed builds.
No configuration switch for number of retries was done on purpose (only
3 retries and no more).
And this is how it looks BTW:
$ gcc ../test/tinytest_demo.c ../test/tinytest.c
$ ./a.out --verbose --no-fork
demo/timeout_retry
demo/timeout_retry:
FAIL ../test/tinytest_demo.c:201: assert(i != 1): 1 vs 1
[timeout_retry FAILED]
[RETRYING timeout_retry (3)]
demo/timeout_retry:
OK ../test/tinytest_demo.c:201: assert(i != 1): 2 vs 1
OK ../test/tinytest_demo.c:213: assert(t2-t1 >= 4): 5 vs 4
OK ../test/tinytest_demo.c:215: assert(t2-t1 <= 6): 5 vs 6
1 tests ok. (0 skipped)
We have calls to the next functions but do not check return values,
though they can be invalid and it is better to show this somehow.
Also do bufferevent_setfd() first and only after it
bufferevent_enable()/bufferevent_disable() since:
a) it is more natural
b) it will avoid extra operations
c) it will not fail first bufferevent_enable() (this is the case for
buffbufferevent_async at least)
In this case we could add more information for issues like #709
* iocp-fixes:
regress: test for HTTP/HTTPS with IOCP enabled
bev_async: trigger/run only deferred callbacks
bev_async: do not initialize timeouts multiple times
bev_async: set "ok" on setfd if fd>=0 (like we do during creation)
bev_async: ignore ERROR_INVALID_PARAMETER on .setfd for iocp
Closes: #709
Refs: nmathewson/Libevent#160
Otherwise callbacks will be runned even without event_loop, due to
nature of IOCP.
A simple example is:
evhttp_connection_free(client)
# freeing the client will trigger evhttp_connection_free() for the
# client on the server side, and hence there will double free
evhttp_free(server)
Fixes: iocp/http/simple
You cannot event_assign() event multiple times, this is UB, and most
likely will fail.
Fixes: af9b2a7ae0be11c79a909d212b1833a9379e4ba0 ("Initialize async
bufferevent timeout CBs unconditionally")
listener already calls event_iocp_port_associate_() the second call will
return ERROR_INVALID_PARAMETER.
Plus we already ignore it on creation, so why we should care about it
here?
Although this is not a problem, since bufferevent uses finalizers and
will free itself only from the loop (well this is not a problem if you
do not play games with various event_base in different threads) it
generates questions, so rewrite it in more reliable way.
Fixes: #712
Some improvements for http-server sample:
- getopt
- persistent port via -p option
- IOCP for win32 via -I
- disable buffering
- enable debug logging via -v/EVENT_DEBUG_LOGGING_ALL
- cleanup (by signal and separate error path on errors)
* sample-http-server:
s/http-server: graceful cleanup
s/http-server: enable debug logging if EVENT_DEBUG_LOGGING_ALL env isset
s/http-server: turn off buffering (otherwise do output on win32)
s/http-server: add an option to use IOCP
s/http-server: add options (for persistent port)
Refs: #709
Since:
- it is not a library
- this file should have (if I had enough time) enough fixes in itself
and should not polute libevent history
- it "requires" (it more cleaner to use it in this way) script --
tools/vagrant-tests.py (indeed, from libevent-extras)
- will has it's own issues/README/...
https://github.com/libevent/libevent-extras
Even after referenced patch there is still possible recursive callbacks
from evbuffer_drain(bev_input), i.e.:
wm_transfer() -> evbuffer_drain() -> wm_transfer()
inc(ctx->get)
But if we will increment ctx->get before drain that we will not add more
data to buffer.
Refs: 54c6fe3c ("regress_ssl: make ssl/bufferevent_wm_filter more fault-tolerance")
CI: https://ci.appveyor.com/project/nmathewson/libevent/build/job/f0rv299i71wnuxdq#L2546
various build checks (i.e. detecting headers/macroses/functions) takes
7 minutes (from 13 minutes in total) for cmake, which is too high.
By using cache we can reduce this to ~0.
And set APPVEYOR_SAVE_CACHE_ON_ERROR so that cmake checks will be
cached (anyway all sources will be built from scratch due to timestamp
updates while extracting from sources).
Otherwise cmake complains:
Policy CMP0075 is not set: Include file check macros honor
CMAKE_REQUIRED_LIBRARIES. Run "cmake --help-policy CMP0075" for policy
details. Use the cmake_policy command to set the policy and suppress this
warning.
CMAKE_REQUIRED_LIBRARIES is set to:
ws2_32.lib
For compatibility with CMake 3.11 and below this check is ignoring it.
We have $env:OPENSSL_ROOT (env) equals to -DOPENSSL_ROOT (cmake
variable) anyway.
cmake complains:
Policy CMP0074 is not set: find_package uses <PackageName>_ROOT variables.
Run "cmake --help-policy CMP0074" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Environment variable OpenSSL_ROOT is set to:
C:/OpenSSL-Win64/bin
For compatibility, CMake is ignoring the variable.
This warning is for project developers. Use -Wno-dev to suppress it.
Due to inplace callbacks (i.e. no BEV_OPT_DEFER_CALLBACKS) we cannot be
sure that wm_transfer() will not be called recursively and indeed it
still happens sometimes, and the referenced patch increase amount of
this times, especially for linux/poll.
Fixes: 66304a23cf748714159c988e78f35401c5352827 ("Fix
ssl/bufferevent_wm_filter when bev does not reach watermark on break")
EVHTTP_CON_READ_ON_WRITE_ERROR works only if an error already read from
the socket, but if we already got EPIPE on write we cannot read from the
socket anymore, and win32 does not guarantee that read will happens
before (although it happens from time to time).
In the referenced patch I just replaced callback with not expecting 417,
but like I already wrote, this is not always true (i.e. it is flacky).
Fixes: 3b581693ac1967f7f8d98491cb772a1b415eb4cd ("test/http:
read_on_write_error: fix it for win32")
* ssl_bufferevent_wm_filter-fix:
Fix ssl/bufferevent_wm_filter when bev does not reach watermark on break
regress_ssl: cover watermarks with deferred callbacks
regress_ssl: improve bufferevent_wm/bufferevent_wm_filter logging
For the ssl/bufferevent_wm* we have next configuration:
- payload_len = 1024
- wm_high = 5120
- limit = 40960
- to_read = 512
In this test we expect that with high watermark installed to "wm_high"
we will read "limit" bytes by reading "to_read" at a time, but adding
"payload_len" at a time (this "to_read"/"payload_len" limits is
installed to finally overflow watermark).
Once we read "limit" bytes we break, by disable EV_READ and reset
callbacks. Although this will not work if when we want to break we do
not reach watermark, this is because watermarks installs evbuffer
callback for the input buffer and if the watermark does not reached it
will enable EV_READ while be_openssl_enable() will read from the
underlying buffer (in case the openssl bufferevent created via
bufferevent_openssl_filter_new()) and call callback again (until it will
reach watermark or read al from the underlying buffer -- this is why it
stops in our caes).
And this is exactly what happened in win32, you can see this in the
following logs:
- win32 before:
OK C:\vagrant\test\regress_ssl.c:829: wm_transfer-client(00DC2750): in: 4608, out: 0, got: 40960
OK C:\vagrant\test\regress_ssl.c:834: wm_transfer-client(00DC2750): break
OK C:\vagrant\test\regress_ssl.c:829: wm_transfer-client(00DC2750): in: 4608, out: 0, got: 41472
OK C:\vagrant\test\regress_ssl.c:834: wm_transfer-client(00DC2750): break
OK C:\vagrant\test\regress_ssl.c:829: wm_transfer-client(00DC2750): in: 4608, out: 0, got: 41984
OK C:\vagrant\test\regress_ssl.c:834: wm_transfer-client(00DC2750): break
OK C:\vagrant\test\regress_ssl.c:829: wm_transfer-client(00DC2750): in: 4608, out: 0, got: 42496
OK C:\vagrant\test\regress_ssl.c:834: wm_transfer-client(00DC2750): break
- win32 after:
OK C:\vagrant\test\regress_ssl.c:821: wm_transfer-client(00FC26F0): break
OK C:\vagrant\test\regress_ssl.c:836: wm_transfer-client(00FC26F0): in: 4800, out: 0, got: 40960
- linux before:
OK ../test/regress_ssl.c:829: wm_transfer-client(0x55555566f5e0): in: 5120, out: 0, got: 40960
OK ../test/regress_ssl.c:834: wm_transfer-client(0x55555566f5e0): break
- linux after:
OK ../test/regress_ssl.c:821: wm_transfer-client(0x55555566f5e0): break
OK ../test/regress_ssl.c:836: wm_transfer-client(0x55555566f5e0): in: 5120, out: 0, got: 40960
(As you can see in linux case we already reach watermark hence it passed
before).
So fix the issue by breaking before draining.
But during fixing this I was thinking is this right? I.e. reading from
the be_openssl_enable(), maybe we should force deferred callbacks at
least?
* check-O_NONBLOCK-in-debug:
regress: use non blocking descriptors whenever it is possible
assert that fds are nonblocking in debug mode
Closes: nmathewson/Libevent#90
Next tests uses fds without O_NONBLOCK flag
- main/free_active_base
- main/many_events
- et/et (has some other bits cleaned up by using TT_* flags and test
setup/cleanup callbacks)
And hence they will fail in debug mode (EVENT_DEBUG_MODE=):
Assertion flags & O_NONBLOCK failed in event_debug_assert_socket_nonblocking_
Check that each fd that had been added with some event do has O_NOBLOCK
after event_enable_debug_mode()
Rebased and do not check signals (EV_SIGNAL) by azat.
Refs: nmathewson/Libevent#90
Refs: #96
* event-ET-#636-v2:
Preserve ET bit for backends with changelist
Epoll ET setting lost with multiple events for same fd
Cover ET with multiple events for same fd
Add ET flag into event_base_dump_events()
Fixes: #636
