test_malloc_shutdown_hang (Debug build): Assertion isMallocInitialized() failed

[phprus]

Commit: 86fe3f0
MSVC: 19.21 32 bit
Windows 7 SP1.

CMake command:

cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -G "NMake Makefiles" ..\..

Error:

F:\tmp\tbb\oneTBB-86fe3f04c1b319faecebdc8b642ecc896fbb2c3b\build\1921d>msvc_19.2
1_cxx17_32_md_debug\test_malloc_shutdown_hang.exe
[doctest] doctest version is "2.4.6"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases: 1 | 1 passed | 0 failed | 0 skipped
[doctest] assertions: 1 | 1 passed | 0 failed |
[doctest] Status: SUCCESS!
Assertion isMallocInitialized() failed (located in the internalPoolFree function, line in file: 2636)

Call Stack:

>   tbbmalloc_debug.dll!rml::internal::assertion_failure_impl(const char * location, int line, const char * expression, const char * comment) Line 51   C++
    tbbmalloc_debug.dll!rml::internal::assertion_failure::__l2::<lambda>() Line 73  C++
    tbbmalloc_debug.dll!tbb::detail::d0::run_initializer<void <lambda>(void)>(const rml::internal::assertion_failure::__l2::void <lambda>(void) & f, std::atomic<enum tbb::detail::d0::do_once_state> & state) Line 289 C++
    tbbmalloc_debug.dll!tbb::detail::d0::atomic_do_once<void <lambda>(void)>(const rml::internal::assertion_failure::__l2::void <lambda>(void) & initializer, std::atomic<enum tbb::detail::d0::do_once_state> & state) Line 277    C++
    tbbmalloc_debug.dll!rml::internal::assertion_failure(const char * location, int line, const char * expression, const char * comment) Line 73    C++
    tbbmalloc_debug.dll!rml::internal::internalPoolFree(rml::internal::MemoryPool * memPool, void * object, unsigned int size) Line 2636    C++
    tbbmalloc_debug.dll!rml::internal::internalFree(void * object) Line 2666    C++
    tbbmalloc_debug.dll!scalable_free(void * object) Line 2944  C++
    tbb12_debug.dll!tbb::detail::r1::deallocate_memory(void * p) Line 225   C++
    tbb12_debug.dll!tbb::detail::d1::tbb_allocator<std::_Tree_node<tbb::detail::d1::global_control *,void *>>::deallocate(std::_Tree_node<tbb::detail::d1::global_control *,void *> * p, unsigned int __formal) Line 65 C++
    [External Code] 
    ucrtbased.dll![Frames below may be incorrect and/or missing, no symbols loaded for ucrtbased.dll]   Unknown

Dump and PDB files in attach:
test_malloc_shutdown_hang.zip

[alexey-katranov]

For some reason tbbmalloc is not initialized while the presumably correct object is freeed. Note that tbbmalloc cannot be de-initialized (when compiled into dynamic library), so it is unclear how the object was allocated without tbbmalloc initialization. Is the issue reproduced each time or is it sporadic issue?

[phprus]

On Windows 7 SP1 - each time.

[alexey-katranov]

It seems your environment is the cornucopia of mysterious issues :). Can you please help to investigate the issue?

Is the issue reproduced when child processes are not created? Can you replace child process spawn with direct call of main test_malloc_shutdown_hang.cpp:65-67:

-void processSpawn(const char* self) {
-    _spawnl(_P_WAIT, self, self, "1", NULL);
-}
+int main(int argc, char* argv[]);
+void processSpawn(const char* ) {
+    //_spawnl(_P_WAIT, self, self, "1", NULL);
+    char* args[2] = { "", "1" };
+    main(2, args);
+}

[phprus]

Windows 7 Professional SP1 installed from scratch + Visual Studio 2019 (with all C++ components) + CMake.
Memory checked by Memtest86+ - no errors.
I'm surprised myself to :)
But my environment helped to find the real error: #553 ;)

After replace to direct call:

...
===============================================================================
[doctest] test cases: 1 | 1 passed | 0 failed | 0 skipped
[doctest] assertions: 1 | 1 passed | 0 failed |
[doctest] Status: SUCCESS!
Assertion isMallocInitialized() failed (located in the internalPoolFree function
, line in file: 2636)

F:\tmp\tbb\oneTBB-86fe3f04c1b319faecebdc8b642ecc896fbb2c3b\build\1921d>

"Assertion failed" is present.

[phprus]

Replace to

void processSpawn(const char* /*self*/) {
    REPORT("Known issue: no support for process spawn on this platform.\n");
    REPORT("done\n");
    exit(0);
}

also raises an error:

Known issue: no support for process spawn on this platform.
done
Assertion isMallocInitialized() failed (located in the internalPoolFree function
, line in file: 2636)

F:\tmp\tbb\oneTBB-86fe3f04c1b319faecebdc8b642ecc896fbb2c3b\build\1921d>

[alexey-katranov]

Can you try the following steps:

1. set breakpoint frontend.cpp:2087 that mallocInitialized is set to 2.
2. If the breakpoint is hit, take the address of mallocInitialized and set a watch (Debug->New breakpoint->Data breakpoint) to query where it is changed to 0.
3. Continue application execution.

[phprus]

Screenshot:

Replace (https://github.com/oneapi-src/oneTBB/blob/master/src/tbbmalloc/frontend.cpp#L1947)

static std::atomic<intptr_t> mallocInitialized{0};   // implicitly initialized to 0

to

static std::atomic<intptr_t> mallocInitialized;

fix this error.

But this change is not valid C++ (https://en.cppreference.com/w/cpp/atomic/atomic/atomic)...

[alexey-katranov]

I would not say that it is not valid C++. Perhaps, it as a grey area but consider the following statements in the C++11 standard

3.6.2:2:

Variables with static storage duration ([basic.stc.static]) or thread storage duration ([basic.stc.thread]) shall be zero-initialized ([dcl.init]) before any other initialization takes place.

26.9.5:4:

A::A() noexcept = default;
Effects: leaves the atomic object in an uninitialized state. [ Note: These semantics ensure compatibility with C. — end note ]

So, if the atomic with static storage duration is default constructed, it should be zero-initialized. I.e. [26.9.5:4] guarantees that zero initialization will not be overwritten with default constructor (while the constructor with 0 does not provide such guarantee).

Thank you for catching one more issue.

[phprus]

But since C++20 std::atomic<>::atomic() is not the default constructor

See:

• http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0883r2.pdf
• <atomic>: In pre-C++20 mode, the constructor should be trivial microsoft/STL#661
• https://bugs.llvm.org/show_bug.cgi?id=45367
• https://codereview.qt-project.org/c/qt/qtbase/+/305484

[alexey-katranov]

But since C++20 std::atomic<>::atomic() is not the default constructor

Strictly speaking, it is default constructor because it "can be called with no arguments" ((cppreference)[https://en.cppreference.com/w/cpp/language/default_constructor]). I believe you mean that it does not perform (default initialization)[https://en.cppreference.com/w/cpp/language/default_initialization] of underlying type. It is Ok in that case because we rely on static zero-initialization.

[phprus]

I misunderstood the statement "[26.9.5: 4] guarantees that zero initialization will not be overwritten with default constructor"
I thought this only applies to the "A::A() = default" case. Not for all default constructors ("can be called with no arguments").

[anton-potapov]

@alexey-katranov , I believe enabliblig C++20 should not break 'tbbmalloc' ?

[alexey-katranov]

As far as I remember, C++20 brings interesting topic of atomic objects initialization and access in data sections (static storage duration). It seems it is not safe to use global atomics and it is better to consider atomic_ref.