search

zeiss-microscopy / libCZI

Open Source Cross-Platform C++ library to read CZI image files
GNU General Public License v3.0
71 stars 19 forks source link

thread safety #39

Closed CHILI-GmbH closed 5 years ago

CHILI-GmbH commented 6 years ago

the docs states that the lib aims to be thread-safe.

does this extend to reading the data from disk? i.e. is it supposed to be possible to use the same cziReader with an opened stream from multiple threads?

doing so gives Illegal data detected at offset 528800 -> Invalid SubBlockk-magic errors.

creating new cziReader objects with new streams in each thread avoids these errors, but incurs a considerable overhead.

ptahmose commented 5 years ago

Yes, it is supposed to be possible to have one cziReader-object and concurrently "read" from it. One loose end I am aware of: this requires a stream-implementation which allows concurrent calls to IStream::Read. In current code, only the version for Win32 (CSimpleStreamImplWindows) supports this. The other implementations (which are used in Linux e.g. CSimpleStreamImpl) are obviously not thread-safe (since they use an fseek and a subsequent fread, which obviously is not a good thing for concurrent access…). A simple fix would be a critical section here, but maybe there is a portable way which is along the lines of the Win32-implementation - I don't know.

CHILI-GmbH commented 5 years ago

we only use linux and mac os. so maybe this is the problem.

i'm now using a thread pool and a czi-reader object per actual thread and this works. this is portable between all platforms and requires no platform dependent code.

as the overhead for the additional readers is not per job but only per thread this is fine for us. without testing: i would think a critical second would impose much more overhead in our case.

maybe this limitation on linux should be documented somewhere.

ptahmose commented 5 years ago

I had a quick look, and I'd guess that "pread"/"pread64" should do the job -> http://man7.org/linux/man-pages/man2/pread.2.html , https://stackoverflow.com/questions/19780919/read-write-from-file-descriptor-at-offset . However - this would probably require some configuration fiddling (since pread seems to be Linux-only?). Yes indeed, this should better be mentioned in the documentation.