Segfault in `convertClearFollower()` when follower exits abruptly

[kadinsayani]

When a dqlite follower node terminates unexpectedly (without a graceful shutdown), convertClearFollower() tries to free an invalid current_leader.address pointer, causing a crash:

#6  0x00007668d86add55 in __GI___libc_free (mem=0x1) at ./malloc/malloc.c:3375
#7  0x00007668d88c0e54 in convertClearFollower (r=0x766840003ce0) at src/raft/convert.c:49

We discovered this segfault during LXD's test_clustering_heal_networks_stop() integration test (prior to canonical/lxd#16806).

core.tar.gz contains the core dump captured at crash.

Additional information:

• dqlite compiled with autoreconf -i && CFLAGS="-g -O0" ./configure --enable-build-raft --enable-debug && make -j
• lxd compiled with CC="cc" CGO_LDFLAGS_ALLOW="(-Wl,-wrap,pthread_create)|(-Wl,-z,now)" go install -v -tags "libsqlite3" -trimpath -gcflags="all=-N -l" ./lxd

[kadinsayani]

Happy to provide debug binaries upon request :)

[tomponline]

@kadinsayani please can you provide these to @marco6 ta

[kadinsayani]

@kadinsayani please can you provide these to @marco6 ta

Shared via google drive.

[marco6]

Given the value passed to raft_free was 0x1 it should likely be solved by #859.

While the bug there was 100% true, testing it is a bit of a pain. @kadinsayani could you try again to see if it fails in your CI instead?

[kadinsayani]

Given the value passed to raft_free was 0x1 it should likely be solved by #859.

While the bug there was 100% true, testing it is a bit of a pain. @kadinsayani could you try again to see if it fails in your CI instead?

I just tested off dqlite main branch, still getting a segfault...

[marco6]

Did you test this locally or on the CI? If on the CI, are you sure it didn't use the cached version?

If not on the CI, Is It Easy to reproduce? Could you run it with tracing enabled?

[kadinsayani]

Did you test this locally or on the CI? If on the CI, are you sure it didn't use the cached version?

Tested locally with a fresh dqlite build.

If not on the CI, Is It Easy to reproduce? Could you run it with tracing enabled?

I just tried building dqlite with --enable-backtracing, and hit this error:

/usr/bin/ld: /root/go/deps/dqlite/.libs//libdqlite.so: undefined reference to `registry__size'

[marco6]

Having spoken in person with @kadinsayani, the build problem was caused by additional cflags passed (-O0 -g together with --enable-debug).
While it is still not clear to me the reason for that, it is clearly not related to the segfault.

In our in person test, the segfault seems to be still there, even though it might be in a different spot after #859.

Last, the behavior is happening in a test where the Unix domain socket used for communication is forcefully deleted on a running machine. While this is not a normal shutdown sequence, it would be best for dqlite not to ever corrupt memory, of course.

[marco6]

As a wrap-up, this is indeed a bug caused by removing the directory before stopping dqlite (it will make set_{term,vote} fail).

But what this exposed was a relatively hard to catch Undefined Behaviour which I am very happy to have removed.

So, thank you for this report @kadinsayani ! This was very useful!

[marco6]

#866 also fixes all the inline misuses I could find in the repo.

[kadinsayani]

Happy to have helped! Thanks for the fix!