fix multi construct of g_gmock_implicit_sequence

[CastleOnTheHill]

When libgmock.a is linked to a multitude of shared libraries, g_gmock_implicit_sequence will be constructed multiple times. For instance:

libgmock.a is linked to libA.so
libgmock.a is linked to libB.so
libA.so and libB.so are linked to BinaryC

When BinaryC executes, g_gmock_implicit_sequence is constructed and destroyed twice, and the private member const pthread_key_t key_ in ThreadLocal will be called in pthread_key_delete twice, resulting in the failure of GTEST_CHECK_POSIX_SUCCESS_(pthread_key_delete(key_)).

Furthermore, ThreadLocal<Sequence*> g_gmock_implicit_sequence is not trivially destructible, which violates the google code style.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[higher-performance]

Hi! I'm a little bit confused, how does moving the global into a function avoid the problem of multiple constructions? Wouldn't each shared library still get its own copy of that global? Perhaps the constructions would be avoided if the function isn't called from both libraries, but if/when it is, you should still get two constructions, no?

It seems to me that what you may really want is for libgmock to be shared rather than static?

[WillAyd]

FWIW I am seeing this same issue when compiling against the googletest source, so I don't think the shared / static conversation has anything to do with it. Having a hard time reproducing an MRE but hope to be able to craft one soon.

Here's some backtraces from gdb to illustrate - the first and the last breakpoints are from an object at the same memory address, trying to delete the same pthread key twice and causing UB

Breakpoint 1, testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764      ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, 
    __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#2  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#3  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#4  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#5  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::~ThreadLocal (this=0x5555557a3d18, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764      ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::~ThreadLocal (
    this=0x5555557a3d18, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00005555556cf74b in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#2  0x00005555556cf870 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#3  0x00005555556cf12e in testing::UnitTest::~UnitTest (this=0x555555790860 <testing::UnitTest::GetInstance()::instance>, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5538
#4  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#5  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#6  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#7  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#8  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::~ThreadLocal (this=0x5555557a3b60, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764      ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::~ThreadLocal (this=0x5555557a3b60, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00005555556cf811 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#2  0x00005555556cf870 in testing::internal::UnitTestImpl::~UnitTestImpl (this=0x5555557a3ad0, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5593
#3  0x00005555556cf12e in testing::UnitTest::~UnitTest (this=0x555555790860 <testing::UnitTest::GetInstance()::instance>, __in_chrg=<optimized out>)
    at ../subprojects/googletest-1.14.0/googletest/src/gtest.cc:5538
#4  0x00007ffff7045495 in __run_exit_handlers (status=0, listp=0x7ffff721a838 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, 
    run_dtors=run_dtors@entry=true) at ./stdlib/exit.c:113
#5  0x00007ffff7045610 in __GI_exit (status=<optimized out>) at ./stdlib/exit.c:143
#6  0x00007ffff7029d97 in __libc_start_call_main (main=main@entry=0x55555570216f <main(int, char**)>, argc=argc@entry=1, argv=argv@entry=0x7fffffffda28)
    at ../sysdeps/nptl/libc_start_call_main.h:74
#7  0x00007ffff7029e40 in __libc_start_main_impl (main=0x55555570216f <main(int, char**)>, argc=1, argv=0x7fffffffda28, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffda18) at ../csu/libc-start.c:392
#8  0x0000555555643725 in _start ()
(gdb) c
Continuing.

Breakpoint 1, testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
1764      ~ThreadLocal() {
(gdb) bt
#0  testing::internal::ThreadLocal<testing::Sequence*>::~ThreadLocal (this=0x555555790900 <testing::internal::g_gmock_implicit_sequence>, 
    __in_chrg=<optimized out>) at ../subprojects/googletest-1.14.0/googletest/include/gtest/internal/gtest-port.h:1764
#1  0x00007ffff7045a56 in __cxa_finalize (d=0x7ffff7de6350) at ./stdlib/cxa_finalize.c:83
#2  0x00007ffff7bb9e77 in __do_global_dtors_aux () from /home/willayd/clones/arrow-adbc/c/builddir/driver/sqlite/../../validation/libadbc_validation.so
#3  0x00007fffffffd8a0 in ?? ()
#4  0x00007ffff7fc924e in _dl_fini () at ./elf/dl-fini.c:142

[higher-performance]

FWIW I am seeing this same issue when compiling against the googletest source, so I don't think the shared / static conversation has anything to do with it.

Not sure if I understand what you're doing correctly, but compiling each of 2 libraries against the source of the same 3rd is equivalent to static linking as far as this problem is concerned, and hence (as expected in my last comment) you'd expect this to come up there too. But yeah, an MRE would be helpful whenever you have one.

[WillAyd]

Ah OK - thanks for that info.

I am using the Meson wrap for googletest which just provides the source file to other compilation targets. I'll see if I can work around that and get a shared library compiled instead, or try to get this down to something more debuggable. Thanks for the quick reply

[WillAyd]

To be clear though - I don't have the same linkage situation as the OP, just am getting the same error. My linker command looks something like:

c++ -o driver/sqlite/adbc-driver-sqlite-test
subprojects_googletest-1.14.0_googletest_src_gtest-all.cc.o
subprojects_googletest-1.14.0_googlemock_src_gmock-all.cc.o
subprojects_googletest-1.14.0_googlemock_src_gmock_main.cc.o
...

[WillAyd]

While not quite an MRE you can reasonably see this if you work with this project:

https://github.com/WillAyd/arrow-adbc/tree/aeccd962d530a61f31909c083a05412110e77f2b

With the following compilation steps:

cd c
meson setup builddir -Dtests=true -Dsqlite=true && cd builddir
meson compile

Then either meson test or ./driver/sqlite/adbc-driver-sqlite-test for an even more minimal interaction

[higher-performance]

Thank you, but I suspect we won't have the bandwidth to to track down such an issue in a massive project like this. I'd really need something minimal that I can understand by eyeballing.

[WillAyd]

Totally understood - no expectations for you to go through those steps. Just providing as reference for now

[higher-performance]

I'm going to close this PR, since I think we need to see something indicating that (a) an issue actually exists inside GoogleTest (as opposed to the issue being that you should be using shared libraries instead of static), and that (b) this PR correctly fixes it. So far neither seems to be the case to me, but if anything comes up indicating otherwise, please do feel free to follow up.

[WillAyd]

FWIW the issue I was posting about before become clearer when compiling with ASAN, which ultimately flagged a violation of ODR based off of how I was linking everything together. Sharing in case that is helpful to others

Thanks @higher-performance for your guidance and keeping this on the right track