Changeset View
Standalone View
src/test/util_tests.cpp
Show First 20 Lines • Show All 1,273 Lines • ▼ Show 20 Lines | while (true) { | ||||
case UnlockCommand: | case UnlockCommand: | ||||
ReleaseDirectoryLocks(); | ReleaseDirectoryLocks(); | ||||
ch = true; // Always succeeds | ch = true; // Always succeeds | ||||
rv = write(fd, &ch, 1); | rv = write(fd, &ch, 1); | ||||
assert(rv == 1); | assert(rv == 1); | ||||
break; | break; | ||||
case ExitCommand: | case ExitCommand: | ||||
close(fd); | close(fd); | ||||
exit(0); | // As an alternative to exit() which runs the exit handlers | ||||
// (which seem to be flakey with Boost test suite with JUNIT | |||||
// logging in a forked process), just vanish this process as | |||||
// fast as possible. This also stops valgrind from looking into | |||||
// this uninteresting subprocess. (quick_exit() would also work, | |||||
// but it triggers a spurious glibc-valgrind error in pre-3.15 | |||||
// versions.) | |||||
execlp("true", (char *)NULL); | |||||
deadalnix: According to https://en.cppreference.com/w/cpp/utility/program/exit , std::exit should run all… | |||||
markblundebergAuthorUnsubmitted Done Inline ActionsYeah the primary problem is C++'s cleanup, specifically involving some static objects created by the Boost test suite logging subsystem. Running valgrind with the command below uncovers that there is a use-after-free during the cleanup, and the actual crashes causing problems in the CI were apparently a double-free "corrupted double-linked list" error also involving Boost cleanup, probably related. It's still unclear why forking would induce such a problem. Using std::abort actually causes test failure since the subprocess now returns nonzero status (the unit test checks for this, BOOST_CHECK_EQUAL(processstatus, 0); -- that's why segfault in subprocess was causing overall test failure), but quick_exit does do the trick. The only thing is that this introduces a new problem - glibc+valgrind have a bug in cleanup that looks like this: https://stackoverflow.com/a/58292789/5667904 . As a side issue, valgrind also produces a memory leak report that complains about the sub process having leaked memory if it doesn't clean up. So what I have here is the cleanest option, as odd as it is, since it basically avoids ALL memory cleanup handlers, both C and C++, and also tells valgrind to not worry about the memory leaks. It is the only option I can find that allows the following to pass test suite and not produce any valgrind complaints: valgrind ./src/test/test_bitcoin --logger=JUNIT -t util_tests markblundeberg: Yeah the primary problem is C++'s cleanup, specifically involving some static objects created… | |||||
FabienUnsubmitted Not Done Inline ActionsIf quick_exit is what we want, it is still possible to use it associated with a suppression file to avoid the glibc bug 24476. { Suppress glibc bug 24476 Memcheck:Addr8 ... fun:__dlerror_main_freeres fun:__libc_freeres ... fun:quick_exit* fun:TestOtherProcess } There is also a Valgrind option to not use the freeres feature at all that prevent the issue to throw: --run-libc-freeres=no but I don't know what could be the consequences when running against other pieces of code (it is advertised as causing false positives). Fabien: If quick_exit is what we want, it is still possible to use it associated with a suppression… | |||||
markblundebergAuthorUnsubmitted Done Inline ActionsYeah the suppressor option seems most ideal for that approach. The only downside in comparison to current diff is that valgrind will still print out a nasty leak report for the subprocess, but as long as we know what is going on that's fine, and it sounds like we can add --child-silent-after-fork=yes though I haven't tested that. markblundeberg: Yeah the suppressor option seems most ideal for that approach. The only downside in comparison… | |||||
FabienUnsubmitted Not Done Inline ActionsSee D5254 for this alternative solution. Fabien: See D5254 for this alternative solution. | |||||
default: | default: | ||||
assert(0); | assert(0); | ||||
} | } | ||||
} | } | ||||
} | } | ||||
#endif | #endif | ||||
BOOST_AUTO_TEST_CASE(test_LockDirectory) { | BOOST_AUTO_TEST_CASE(test_LockDirectory) { | ||||
▲ Show 20 Lines • Show All 198 Lines • Show Last 20 Lines |
According to https://en.cppreference.com/w/cpp/utility/program/exit , std::exit should run all the proper exit code. execlp probably not - no C lib call would guarantee that.
Not sure if naked exit is the same as std::exit , I assume the libc doesn't guarantee C++'s cleanup. If the problem is C++'s cleanup, then this is all good. If the goal is to exit WITHOUT cleaning up, then std::abort is what you want.