googletest icon indicating copy to clipboard operation
googletest copied to clipboard

Preprocessor macros GTEST_IS_THREADSAFE and GTEST_HAS_PTHREAD do not work as expected

Open ingo-h opened this issue 3 years ago • 9 comments

Describe the bug

At ./googletest/README.md (not at ./README.md) you find:

Multi-threaded Tests

GoogleTest is thread-safe where the pthread library is available. After #include "gtest/gtest.h", you can check theGTEST_IS_THREADSAFE macro to see whether this is the case (yes if the macro is #defined to 1, no if it's undefined.).

If GoogleTest doesn't correctly detect whether pthread is available in your environment, you can force it with

-DGTEST_HAS_PTHREAD=1

or

-DGTEST_HAS_PTHREAD=0

GTEST_IS_THREADSAFE always compiles to "yes" (macro is defined) no matter if PThreads are available or not.

cmake option -DGTEST_HAS_PTHREAD has no effect no matter if set to 0 or 1.

I expect that both macros work as documented.

Steps to reproduce the bug

Check GTEST_IS_THREADSAFE on every compiler that supports preprocessor macros, for example with GCC:

#ifdef GTEST_IS_THREADSAFE
#warning "pthread is available"
#else
#warning "pthread is NOT available"
#endif

always compiles to

/home/ingo/devel/googletest/test_simple.cpp:18:2: warning: #warning "pthread is available" [-Wcpp]
   18 | #warning "pthread is available"
      |  ^~~~~~~

With GTEST_HAS_PTHREAD try to configure:

~$ cmake -S . -B build -DGTEST_HAS_PTHREAD=0|1

Alternative you may compare configuring with -D gtest_disable_pthreads=ON|OFF

Does the bug persist in the most recent commit?

Yes.

What operating system and version are you using?

Any operating system that supports a compiler that can expand preprocessor macros.

What compiler and version are you using?

~$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/10/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa:hsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 10.2.1-6' --with-bugurl=file:///usr/share/doc/gcc-10/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-10 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-10-Km9U7s/gcc-10-10.2.1/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-10-Km9U7s/gcc-10-10.2.1/debian/tmp-gcn/usr,hsa --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu --with-build-config=bootstrap-lto-lean --enable-link-mutex
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 10.2.1 20210110 (Debian 10.2.1-6)

What build system are you using?

~$ cmake --version
cmake version 3.18.4

CMake suite maintained and supported by Kitware (kitware.com/cmake).

ingo-h avatar Sep 27 '21 10:09 ingo-h

Can I ask why you are trying to disable this?

The non-thread-safe options appear to be untested and we should probably just remove them.

derekmauro avatar Sep 27 '21 19:09 derekmauro

Now I see https://github.com/google/googletest/issues/3577. We probably need to make this configuration work.

derekmauro avatar Sep 27 '21 19:09 derekmauro

@derekmauro Hi Derek,

Can I ask why you are trying to disable this?

It is more a general design decision after having also trouble with sophisticated mocking of pthread functions from the standard library (no dependency injection possible) with segfaults. Then I realized these troubles with GoogleTest. With my multi platform library I would like to have the same configuration on all platforms. To have a defined state on GoogleTest and to reduce side effects and dependencies on tests with pthread functions the idea is to disable pthreads on GoogleTest.

The non-thread-safe options appear to be untested and we should probably just remove them.

Simply removing it from the documentation would be sufficient from my user's point of view.

ingo-h avatar Sep 27 '21 22:09 ingo-h

GTEST_IS_THREADSAFE always compiles to "yes" (macro is defined) no matter if PThreads are available or not.

And I believe the culprit is here the GTEST_HAS_PTHREAD

https://github.com/google/googletest/blob/1b2606425c4040cacadaa22689423ec0a29f316d/googletest/include/gtest/internal/gtest-port.h#L754-L757

cmake option -DGTEST_HAS_PTHREAD has no effect no matter if set to 0 or 1

We can see in this issue https://github.com/google/googletest/issues/3692 that GTEST_HAS_PTHREAD does not honor the cmake option -DGTEST_HAS_PTHREAD rather defines itself the following way:

https://github.com/google/googletest/blob/1b2606425c4040cacadaa22689423ec0a29f316d/googletest/include/gtest/internal/gtest-port.h#L544-L548

@ingo-h Could you please confirm if this thread https://github.com/google/googletest/pull/3693 will resolve this issue?

joshiayush avatar Jan 01 '22 14:01 joshiayush

@joshiayush I will look at it the next days (I'm just a bit busy with another project).

ingo-h avatar Jan 01 '22 15:01 ingo-h

@ingo-h did you get a chance to get back to this?

dalg24 avatar Feb 15 '22 14:02 dalg24

I think things are working better now.

maryfranharper avatar Feb 15 '22 22:02 maryfranharper

I think things are working better now.

Can you elaborate? Do you mean issue is resolved in main?

dalg24 avatar Feb 15 '22 22:02 dalg24

I still see an issue with this if I try to compile GoogleTest without PTHREAD support, even after applying #3693

ciband avatar Aug 24 '23 19:08 ciband