BaseBitmap.Init("somefile.exr") crashes with multiple std::thread

Aaron

Hello colleagues,
I have found an issue that when we want to read several .EXR files in parallel using multiple std::threads they crash on BaseBitmap.Init(file.EXR).
It doesn't crash when we read exr sequentially without threads. It doesn't crash when we use C4DThread instead, however some bitmaps are not read correctly and return err codes. However to perform correctly we need to use C4DThread and mutex lock/unlock before/after bitmap.Init(exr)
You may test this short code. I have tested 4 different 8K .exr files. It reproduces the crash always. Maybe I miss something, some threading parameter for this case.

void bthread(int id)
{
	String st[4] = {"S:\\test\\img1_8K.exr",
					"S:\\test\\img2_8K.exr",
					"S:\\test\\img3_8K.exr",
					"S:\\test\\img4_8K.exr" };

	Filename fn(st[id]);

	BaseBitmap* bitmapE = BaseBitmap::Alloc();
	IMAGERESULT res = bitmapE->Init(fn);
	printf("\n [file %s result %d size %d x %d]", st[id].GetCStringCopy(), res, bitmapE->GetBw(), bitmapE->GetBh());
	BaseBitmap::Free(bitmapE);
}

void test_exr()
{
	std::thread* t[4];
	for (int i = 0; i < 4; i++)
	{
		t[i] = new std::thread(bthread, i);
		t[i]->detach();
	}
}

ferdinand

Hey @Aaron,

Thank you for reaching out to us. You cannot use std in Cinema 4D projects, it is at least is strongly discouraged. All modules (i.e., plugin binaries) have by default exception handling disabled, so std code can easily lead to undefined behaviour. Your code is very non-conformant to the Cinema API (raw arrays, printf, manual heap allocation, std). Unless I am overlooking something, I would also say that your code leaks, because you heap allocate t[i] but never free it.

You can technically turn exceptions on again in our project configurations, but there is a reason why Cinema 4D avoids this (primarily - exceptions are slow). There are some modules/frameworks which have exceptions enabled and which use std (e.g., the misc.framework). But we are then usually very particular in how we use std.

It is also important that you use our threading system, or at least wrap alien threads. When you want to use our types such as BaseBitmap or functions like IO, you must use our threads.

... however some bitmaps are not read correctly and return err codes ...

Well, without the errors and the concrete code I cannot help you much. I would probably also need the exact files which fail.

Cheers,
Ferdinand

Aaron

Ferdinand, thank you for very quick reply.
Of course I know about this sample code with leaks, it was just to keep the message very short.
It's working code for jpg, png, bmp - it scales well on multi-core, reads data correctly, never had a problem with that formats. But not for exr, no any exr in multi-threaded environment.
Actually all exr files fail, you may check. I have catched it on 8K exr files.
It was IMAGERESULT::MISC_ERROR (-6) error code that I mentioned for situation of parallel BaseBitmap.Init() without using mutex to access this function.

Sure I have to use C4D threads to do this kind of optimization. Thank you very much!
I was just hoping to figure out maybe some special image IO mutex lock may help.

Best regards,
Aaron

ferdinand

Hey @Aaron,

I appreciate efforts to keep example code short. I just point out things where I see them. The memory leak is only a minor aspect, which is why I brought it up last. More problematic is that you lean so much into the std library. That will always cause issues when you are not very careful. I short, you can effectively only use parts of std that are noexcept, and even then you can run into problems, as things might work on one OS but not another.

MISC_ERROR is unfortunately a very generic error, as it gets generated like this:

So, this hints at a more fundamental issue, as for example a locked file handle or the OS somehow intervening. You should keep in mind that not all file IO operations are thread safe. E.g., read access to the same file form multiple threads can be non-thread safe. But an SSD can still only read things one by one, so unless your data is already cached, the pure reading cannot be threaded. When you are loading many 8k 32bit EXRs (~570 MB a piece), then you could look easily at multiple seconds of disk IO with a modern NVMe SSD (Gen 4 or 5).

What can be of course threaded and makes sense, is the deserialization of the data on disk, which I assume is your goal here. But does the actual deserialization of EXR images really take that long?

But without concrete code of what you are doing and example data I cannot help you. From what I understand, you currently have multiple threads and just make them wait for each other by using a semaphore? Which probably means that you do not want to use threads to speed up reading, but just happen to have to read from multiple threads?

C4DThread is a very ancient threading mechanism in our API (which is in fact deprecated). The Maxon API has more modern threading mechanisms. You could for example use a jobs group to read your images one by one in a job group and do this from one to many threads.

Cheers,
Ferdinand

Aaron

Ferdinand, thank you very much for detailed explanation!
Sure I will integrate the most recent threading mechanisms. The files that we tried to read in parallel are different. I guess there is something with OpenEXR reader that you probably wrap around. Other file types reading in parallel is working just fine.
The whole code workflow is more complex because it involves several libraries and plugin system communication.
So I have reduced the issue just that short and it's reproduces always.
My goal was indeed to read in parallel, it's not very fast to read and decode sequentially when you deal with 1000s of files.
However for some file types we can probably serialize the access.
Best regards,
Aaron

Aaron

Few more experiments:

When I replace std::thread to omp #parallel for the result is the same - crash
When I launch a std::thread and from inside it I launch several C4D threads each with BaseBitmap.Init() then it works just fine.

I honestly think that BaseBitmap.Init() should be local and independent function from any other C4D infrastructure.

ferdinand

@Aaron said in BaseBitmap.Init("somefile.exr") crashes with multiple std::thread:

When I replace std::thread to omp #parallel for the result is the same

I am not quite sure what 'omp #parallel' is supposed to be? maxon::ParallelFor? Unless it was unclear, there is of course the possibility that BaseBimtap::Init is in principle not thread safe, at least in the context for the EXR bitmap loader (as a general statement that is for sure not true). But gereally without having had a closer look, that strikes me as less likely.

I honestly think that BaseBitmap.Init() should be local and independent function from any other C4D infrastructure.

We do not support std and this unfortunately cannot change. That we do not support std is a not a 'oops, now it is like this' thing but a deliberate decision. Because not all compilers always support the entirety of std and std does not always behave the same with all compilers. So, we will never deliberately move towards std.

When you do not want to drop your std thread, you can try to assimilate it as I linked to in my first posting. But there is then still no guarantee that this will work. BaseBimtap is also a Cinema API entity, and just an adapter for ImageInterface from the Maxon API. You can always access the underlying ImageRef for a BaseBitmap, but without copying data, you cannot construct a BaseBitmap for an existing ImageRef.

Our Image API is very abstracted and aimed at parallelization under the hood. So, what you are trying to do, should be possible in principle, unless there is somewhere a bug or design decision in the EXR loader that makes it not thread safe.

You seem insistent on using std::thread and possibly std in general. As lined out before, I understand that there can be scenarios where it is at least not so easy to move away from std for third parties, but it is simply not supported in our API. When you do not absolutely need std, you will do not do yourself any favours by keep using it. When you still have issues with this, please provide a short compileable code example. I will then have a look (but it might take some time, as we are in the midst of release preparations, and I am quite busy).

Cheers,
Ferdinand

edit: please also provide some sample images then, because there is no guarantee that this will happen with all images.

Aaron

@ferdinand said in BaseBitmap.Init("somefile.exr") crashes with multiple std::thread:

omp #parallel

Sorry for late reply, I am not advocating for std::threads, I understand your design explanation.
The omp #parallel thing is OpenMP directive to launch parallel threads. It's an old but simple method.
Actually I have made a solution by exploiting Cinema way of launching threads and it works fine.
However, I hope it would be good to make BaseBitmap->Init a thread safe and independent call from other C4D threads and resources.
I will collect an example for you with exr images soon.

Cheers,
Aaron