ImagingDataCommons / libdicom

C library for reading DICOM files
https://libdicom.readthedocs.io
MIT License
16 stars 8 forks source link

Errors while reading frames multithreaded #76

Closed mollyclaretechcyte closed 11 months ago

mollyclaretechcyte commented 11 months ago

During attempting to read frames in a multithreaded context, we are seeing unexpected errors. When requesting 2 crops asynchronously from the same DcmFilehandle, they both return errors. When requesting crops in a synchronous manner (also included in the test project for sanity) we do not receive errors. For multithreading the errors are different on subsequent runs, but here are a few of the errors we receive: Reading of Data Element header failed Tag 56494445 cannot have VR '\P' Unable to seek file Unable to seek /tmp/test_data/DCM_0.dcm - Invalid argument DcmError set twice

We are using test files from Openslide: CMU-1-JP2K-33005.zip (DCM_0.dcm is hardcoded in the test program, after extraction).

Demonstration project: https://github.com/mollyclaretechcyte/threaded_libdicom_test/blob/master/dicom-test.cpp

Does libdicom support multithreading like this? Is there something we are doing wrong? Or is this a bug with libdicom?

jcupitt commented 11 months ago

Hi @mollyclaretechcyte,

There's a section in the docs on this:

https://libdicom.readthedocs.io/en/latest/usage.html#thread-safety

But now I look it's confusingly worded and doesn't answer your question. I'll fix it, thanks for pointing this out.

By design, libdicom has no dependencies, not even on a threading library. This means it can't be threadsafe, since it can't lock any internal datastructures. However, there are no global structures, so as long as you don't share a DcmFilehandle between threads, you're fine.

You can share DcmFilehandle between threads if you lock around calls into libdicom. The lock only needs to be per-DcmFilehandle, you don't need a global lock. This is what openslide4 and vipsdisp do.

You might find it more convenient to use libdicom via openslide, unless you need access to the compressed image data.

jcupitt commented 11 months ago

OK, docs updated, and I credited you. Thanks again!

mollyclaretechcyte commented 11 months ago

Thank you @jcupitt !