search

AcademySoftwareFoundation / openexr

The OpenEXR project provides the specification and reference implementation of the EXR file format, the professional-grade image storage format of the motion picture industry.
http://www.openexr.com/
BSD 3-Clause "New" or "Revised" License
1.63k stars 619 forks source link

Crash when using threads with OPENEXR_ENABLE_THREADING=OFF #1902

Open darbyjohnston opened 6 days ago

darbyjohnston commented 6 days ago

Hi, I have been compiling OpenEXR with the option OPENEXR_ENABLE_THREADING=OFF, and have been seeing occasional crashes in the ImfHeader code. My application is already multi-threaded so I thought turning off threads in OpenEXR would be OK. The documentation for the option in OpenEXRSetup.cmake says:

# Whether to enable threading. This can be disabled, although thread pool and tasks
# are still used, just processed immediately
option(OPENEXR_ENABLE_THREADING "Enables threaded processing of requests" ON)

However it looks like the option also removes a mutex in ImfHeader.cpp that is protecting some static data:

#if ILMTHREAD_THREADING_ENABLED
    std::mutex _mutex;
#endif

I think without this mutex multiple threads are accessing static data which causes the crash.

I created a small test program that uses std::async to open a number of EXR files in multiple threads. With OPENEXR_ENABLE_THREADING=OFF it crashes fairly quickly with this trace:

* thread #2, stop reason = EXC_BAD_ACCESS (code=1, address=0x21)
  * frame #0: 0x00000001007df2cc libOpenEXR-3_2_d.31.dylib`std::__1::less<void const*>::operator(this=0x0000000100a78748, __x=0x0000000000000021, __y=0x000000016ff12638)[abi:v15006](void const* const&, void const* const&) const at operations.h:372:17
    frame #1: 0x00000001007df274 libOpenEXR-3_2_d.31.dylib`std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, std::__1::less<void const*>, true>::operator(this=0x0000000100a78748, __x=0x0000000000000021, __y=0x000000016ff12638)[abi:v15006](std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord> const&, void const* const&) const at map:595:17
    frame #2: 0x00000001007df0e4 libOpenEXR-3_2_d.31.dylib`std::__1::__tree_iterator<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, std::__1::__tree_node<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, void*>*, long> std::__1::__tree<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, Imf_3_2::(this=0x0000000100a78738, __v=0x000000016ff12638, __root=0x0000000000000001, __result=0x0000000100a78740)::CompressionRecord>, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord> > >::__lower_bound<void const*>(void const* const&, std::__1::__tree_node<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, void*>*, std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>*) at __tree:2536:14
    frame #3: 0x00000001007defb0 libOpenEXR-3_2_d.31.dylib`std::__1::__tree_iterator<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, std::__1::__tree_node<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, void*>*, long> std::__1::__tree<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord>, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, Imf_3_2::(this=0x0000000100a78738, __v=0x000000016ff12638)::CompressionRecord>, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord> > >::find<void const*>(void const* const&) at __tree:2465:20
    frame #4: 0x00000001007de56c libOpenEXR-3_2_d.31.dylib`std::__1::map<void const*, Imf_3_2::(anonymous namespace)::CompressionRecord, std::__1::less<void const*>, std::__1::allocator<std::__1::pair<void const* const, Imf_3_2::(anonymous namespace)::CompressionRecord> > >::find[abi:v15006](this=0x0000000100a78738 size=4, __k=0x000000016ff12638) at map:1451:68
    frame #5: 0x00000001007d948c libOpenEXR-3_2_d.31.dylib`Imf_3_2::(anonymous namespace)::copyCompressionRecord(dst=0x000060000020c080, src=0x000000016ff12ab0) at ImfHeader.cpp:190:28
    frame #6: 0x00000001007d8e58 libOpenEXR-3_2_d.31.dylib`Imf_3_2::Header::Header(this=0x000060000020c080, other=0x000000016ff12ab0) at ImfHeader.cpp:356:5
    frame #7: 0x00000001007d958c libOpenEXR-3_2_d.31.dylib`Imf_3_2::Header::Header(this=0x000060000020c080, other=0x000000016ff12ab0) at ImfHeader.cpp:349:1
    frame #8: 0x0000000100813d4c libOpenEXR-3_2_d.31.dylib`void std::__1::allocator<Imf_3_2::Header>::construct[abi:v15006]<Imf_3_2::Header, Imf_3_2::Header const&>(this=0x0000600002904060, __p=0x000060000020c080, __args=0x000000016ff12ab0) at allocator.h:165:28
    frame #9: 0x0000000100813cac libOpenEXR-3_2_d.31.dylib`void std::__1::allocator_traits<std::__1::allocator<Imf_3_2::Header> >::construct[abi:v15006]<Imf_3_2::Header, Imf_3_2::Header const&, void>(__a=0x0000600002904060, __p=0x000060000020c080, __args=0x000000016ff12ab0) at allocator_traits.h:290:13
    frame #10: 0x0000000100813bbc libOpenEXR-3_2_d.31.dylib`void std::__1::vector<Imf_3_2::Header, std::__1::allocator<Imf_3_2::Header> >::__push_back_slow_path<Imf_3_2::Header const&>(this=0x0000600002904050 size=0, __x=0x000000016ff12ab0) at vector:1537:5
    frame #11: 0x000000010080f468 libOpenEXR-3_2_d.31.dylib`std::__1::vector<Imf_3_2::Header, std::__1::allocator<Imf_3_2::Header> >::push_back[abi:v15006](this=0x0000600002904050 size=0, __x=0x000000016ff12ab0) at vector:1553:9
    frame #12: 0x000000010080db90 libOpenEXR-3_2_d.31.dylib`Imf_3_2::MultiPartInputFile::initialize(this=0x0000600000010000) at ImfMultiPartInputFile.cpp:328:25
    frame #13: 0x000000010080d76c libOpenEXR-3_2_d.31.dylib`Imf_3_2::MultiPartInputFile::MultiPartInputFile(this=0x0000600000010000, fileName="0001.exr", numThreads=0, reconstructChunkOffsetTable=true) at ImfMultiPartInputFile.cpp:104:9
    frame #14: 0x000000010080e568 libOpenEXR-3_2_d.31.dylib`Imf_3_2::MultiPartInputFile::MultiPartInputFile(this=0x0000600000010000, fileName="0001.exr", numThreads=0, reconstructChunkOffsetTable=true) at ImfMultiPartInputFile.cpp:100:1
    frame #15: 0x000000010083279c libOpenEXR-3_2_d.31.dylib`Imf_3_2::RgbaInputFile::RgbaInputFile(this=0x000000016ff12e30, partNumber=0, name="0001.exr", numThreads=0) at ImfRgbaFile.cpp:1121:27
    frame #16: 0x00000001008329fc libOpenEXR-3_2_d.31.dylib`Imf_3_2::RgbaInputFile::RgbaInputFile(this=0x000000016ff12e30, partNumber=0, name="0001.exr", numThreads=0) at ImfRgbaFile.cpp:1125:1
    frame #17: 0x00000001008329b8 libOpenEXR-3_2_d.31.dylib`Imf_3_2::RgbaInputFile::RgbaInputFile(this=0x000000016ff12e30, name="0001.exr", numThreads=0) at ImfRgbaFile.cpp:1100:7
    frame #18: 0x00000001000085dc bug`main::$_0::operator(this=0x0000600003500140)() const at main.cpp:27:40
    frame #19: 0x0000000100008564 bug`decltype(__f=0x0000600003500140)()) std::__1::__invoke[abi:v15006]<main::$_0>(main::$_0&&) at invoke.h:394:23
    frame #20: 0x0000000100008540 bug`void std::__1::__async_func<main::$_0>::__execute<>(this=0x0000600003500140, (null)=__tuple_indices<> @ 0x000000016ff12eaf) at future:2169:16
    frame #21: 0x0000000100008518 bug`std::__1::__async_func<main::$_0>::operator(this=0x0000600003500140)() at future:2162:16
    frame #22: 0x000000010000819c bug`std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::__execute(this=0x00006000035000b0) at future:1008:9
    frame #23: 0x0000000100009194 bug`decltype(__f=0x00006000002041e8, __a0=0x00006000002041f8).*std::declval<void (std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::*)()>()()) std::__1::__invoke[abi:v15006]<void (std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >*, void>(void (std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::*&&)(), std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >*&&) at invoke.h:359:23
    frame #24: 0x00000001000090d4 bug`void std::__1::__thread_execute[abi:v15006]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >*, 2ul>(__t=size=3, (null)=__tuple_indices<2UL> @ 0x000000016ff12f7f)(), std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >*>&, std::__1::__tuple_indices<2ul>) at thread:290:5
    frame #25: 0x0000000100008a0c bug`void* std::__1::__thread_proxy[abi:v15006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<main::$_0> >*> >(__vp=0x00006000002041e0) at thread:301:5
    frame #26: 0x000000018d9cbfa8 libsystem_pthread.dylib`_pthread_start + 148

This is the test code:

#include <ImfRgbaFile.h>

#include <filesystem>
#include <future>
#include <vector>

int main(int argc, char** argv)
{
    std::vector<std::filesystem::path> inputs;
    for (int i = 1; i < argc; ++i)
    {
        inputs.push_back(argv[i]);
    }

    while (1)
    {
        std::vector<std::future<void> > futures;
        for (size_t i = 0; i < inputs.size(); ++i)
        {
            const std::filesystem::path input = inputs[i];
            std::cout << input.string() << std::endl;

            futures.push_back(std::async(
                std::launch::async,
                [input]
                {
                    Imf::RgbaInputFile file(input.string().c_str());
                }));
        }

        for (size_t i = 0; i < futures.size(); ++i)
        {
            futures[i].get();
        }
    }
    return 0;
}

This is with OpenEXR v3.2.4 on macOS 13.7.

For now I can work around the issue by setting OPENEXR_ENABLE_THREADING=ON and Imf::setGlobalThreadCount(0);.

kdt3rd commented 4 days ago

yes, the OPENEXR_ENABLE_THREADING continues to be maintained more for an embedded device scenario. The preferred solution if you are using in a threaded context is to leave it enabled and do as you do and setGlobalThreadCount to 0, or to do something like what I've proposed to switch the default threadpool to tbb in #1852 (we will include that option in the next non-patch release), or to replace the threadpool with your own, which is also now an option...

darbyjohnston commented 4 days ago

It would be nice to add a note about the thread safety in the option's comments, it took me awhile to track this down. I can create a small PR for it.

kdt3rd commented 2 days ago

Thanks, that would be awesome, it would be better to be clearer that it disables any use of threading primitives, rendering the library potentially unsafe. A few releases from now, I hope to coalesce and remove some of that static data, perhaps allowing a lock-free type extension system to replace it... thanks in advance for submitting a PR.

darbyjohnston commented 17 hours ago

I just added a small comment that the library may not be thread-safe when the option is disabled: https://github.com/AcademySoftwareFoundation/openexr/pull/1908