| Age | Commit message (Collapse) | Author |
|---|
|
For reasons not yet diagnosed, specifically in Mac Release builds, the tests
in test_httprequest.hpp consistently crash with a backtrace suggesting that
the worker thread is calling LLCore::HttpLibcurl::completeRequest() after the
foreground thread calls HttpRequest::destroyService().
Weirdly, even executing a tut::skip() call in every test<n>() function up to
the point of the crash does not eliminate the crash.
|
|
NickyD discovered that the substitute default allocator used for llcorehttp
tests was returning badly-aligned storage, which caused access violations on
alignment-sensitive data such as std::atomic. Thanks Nicky!!
Moreover, the llcorehttp test assertions regarding memory usage, well-
intentioned though they are, have been causing us trouble for years. Many have
already been disabled.
The problem is that use of test_allocator.h affected *everything* defined with
that header file's declarations visible. That inevitably included specific
functions in other subsystems. Those functions then (unintentionally) consumed
the special allocator, throwing off the memory tracking and making certain
memory-related assertions consistently fail.
This is a particular, observable bad effect of One Definition Rule violations.
Within a given program, C++ allows multiple definitions for the same entity,
but requires that all such definitions be the same. Partial visibility of the
global operator new() and operator delete() overrides meant that some
definitions of certain entities used the default global allocator, some used
llcorehttp's. There may have been other, more subtle bad effects of these ODR
violations.
If one wanted to reimplement verification of the memory consumption of
llcorehttp classes:
* Each llcorehttp class (for which memory tracking was desired) should declare
class-specific operator new() and operator delete() methods. Naturally,
these would all consume a central llcorehttp-specific allocator, but that
allocator should *not* be named global operator new().
* Presumably that would require runtime indirection to allow using the default
allocator in production while substituting the special allocator for tests.
* Recording and verifying the memory consumption in each test should be
performed in the test-object constructor and destructor, rather than being
sprinkled throughout the test<n>() methods.
* With that mechanism in place, the test object should provide methods to
adjust (or entirely disable) memory verification for a particular test.
* The test object should also provide a "yes, we're still consuming llcorehttp
memory" method to be used for spot checks in the middle of tests -- instead
of sprinkling in explicit comparisons as before.
* In fact, the llcorehttp test object in each test_*.hpp file should be
derived from a central llcorehttp test-object base class providing those
methods.
|
|
|
|
llcorehttp's test_allocator.{h,cpp} overrides global operator new(), operator
new[](), operator delete() and operator delete[](). The two operator new()
functions used to be declared with throw(std::bad_alloc). Worse, for VS 2013
and previous, we needed _THROW0() and _THROW1(std::bad_alloc) instead,
requiring #if logic.
But with dynamic throw declarations deprecated, we must actually remove those.
That obviates the THROW_BAD_ALLOC() / THROW_NOTHING() workarounds in
test_allocator.cpp.
|
|
In three different places we use the same pattern: an ssl_thread_id_callback()
function (a static member of LLCrashLogger, in that case) that used to be
passed to CRYPTO_set_id_callback() and therefore returned an unsigned long
representing the ID of the current thread.
But GetCurrentThread() is a HANDLE, an alias for a pointer, and you can't
uniquely cram a 64-bit pointer into an unsigned long.
Fortunately OpenSSL has a more modern API for retrieving thread ID. Pass
each ssl_thread_id_callback() function to CRYPTO_THREADID_set_callback()
instead, converting it to accept CRYPTO_THREADID* and call
CRYPTO_THREADID_set_pointer() or CRYPTO_THREADID_set_numeric() as appropriate().
|
|
|
|
|
|
TeamCity
|
|
|
|
|
|
|
|
|
|
|
|
Turns out that Monty didn't intend for the int-flavored representation of
HttpStatus to expand to 64 bits even when unsigned long is that wide. So
change the implicit conversion operator, and its uses, to U32 instead. That
produces a consistent toHex() result for both 32-bit and 64-bit builds.
|
|
|
|
|
|
|
|
This is the function in indra/llmessage/tests/testrunner.py that iterates
through ports in a specified range, looking for an available one. Other
platforms understand a specification of port 0 to mean: "You pick one. I'll
just use whichever one you picked."
|
|
|
|
But since the real problem is quite different, try with that suspected test
restored.
|
|
|
|
Instead of having testrunner.run()'s caller pass a Thread object on which to
run the caller's server instance's serve_forever() method, just pass the
server instance. testrunner.run() now constructs the Thread. This API change
allows run() to also call shutdown() on the server instance when done, and
then join() the Thread.
The hope is that this will avoid the Python runtime forcing the process
termination code to 1 due to forcibly killing the daemon thread still running
serve_forever().
While at it, eliminate calls to testrunner.freeport() -- just make the runtime
pick a suitable port instead.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Loath though I am to skip testing, this consistent failure is not a failure in
the software being tested (llcorehttp) but rather in the dummy server with
which we're testing it.
|
|
It's possible that raising an exception in a worker thread -- even though
we're TRYING to suppress it -- is what's causing the process to terminate with
nonzero rc.
|
|
|
|
llcorehttp/tests had a clone of llmessage/tests/testrunner.py that was almost
identical save for recognizing an extra optional parameter. Migrate those few
lines into llmessage/tests/testrunner.py; eliminate the copy in llcorehttp;
help test_llcorehttp_peer.py find the testrunner.py in llmessage/tests.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
shared pointers. Removed direct cast and dereference of handles.
|
|
|
|
|
|
|
|
and use it for existing LLSomeClass::cleanupClass() calls.
This logs the fact of making the call, as well as making it.
|
|
|
