Fix #106977: Crash when OpenEXR IO fails #107184

Sergey Sharybin · 2023-04-20T17:44:39+02:00

Sergey Sharybin commented

2023-04-20 17:44:39 +02:00

The crash can occur in the following situations:

Attempt to open a corrupted EXR file
Attempt to save an EXR file under a non-existing directory.

The root cause is not really clear: for some reason the OpenEXE API on
the Blender side can not catch OpenEXE exceptions by a constant
reference to a std::exception, although it can by a constant reference
to an Iex::BaseExc.

This does not seem to be an issue with the OpenEXR library itself as
the idiff tool from our SVN folder catches the exceptions correctly.
It is also not caused by the symbols_apple.map as erasing it does not
make the problem go away.

It could still be some compiler/visibility flag which we were unable
to nail down yet.

The proposed solution is to add catch-all cases, mimicking the OIIO
tools. This solves the problem with the downside is that there are
no friendly error messages in the terminal. Those messages could be
brought as part of the workaround by additionally catching the
Iex::BaseExc exception. But probably nobody relies on those error
prints anyway, so added complexity in the code is likely does not
worth it.

The crash can occur in the following situations: - Attempt to open a corrupted EXR file - Attempt to save an EXR file under a non-existing directory. The root cause is not really clear: for some reason the OpenEXE API on the Blender side can not catch OpenEXE exceptions by a constant reference to a std::exception, although it can by a constant reference to an Iex::BaseExc. This does not seem to be an issue with the OpenEXR library itself as the idiff tool from our SVN folder catches the exceptions correctly. It is also not caused by the symbols_apple.map as erasing it does not make the problem go away. It could still be some compiler/visibility flag which we were unable to nail down yet. The proposed solution is to add catch-all cases, mimicking the OIIO tools. This solves the problem with the downside is that there are no friendly error messages in the terminal. Those messages could be brought as part of the workaround by additionally catching the Iex::BaseExc exception. But probably nobody relies on those error prints anyway, so added complexity in the code is likely does not worth it.

Sergey Sharybin added the

Module

Core

label 2023-04-20 17:44:39 +02:00

Sergey Sharybin added 1 commit 2023-04-20 17:44:46 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

de1aee3388 Fix #106977 : Crash when OpenEXR IO fails

The crash can occur in the following situations:

- Attempt to open a corrupted EXR file
- Attempt to save an EXR file under a non-existing directory.

The root cause is not really clear: for some reason the OpenEXE API on
the Blender side can not catch OpenEXE exceptions by a constant
reference to a std::exception, although it can by a constant reference
to an Iex::BaseExc.

This does not seem to be an issue with the OpenEXR library itself as
the idiff tool from our SVN folder catches the exceptions correctly.
It is also not caused by the symbols_apple.map as erasing it does not
make the problem go away.

It could still be some compiler/visibility flag which we were unable
to nail down yet.

The proposed solution is to add catch-all cases, mimicking the OIIO
tools. This solves the problem with the downside is that there are
no friendly error messages in the terminal. Those messages could be
brought as part of the workaround by additionally catching the
Iex::BaseExc exception. But probably nobody relies on those error
prints anyway, so added complexity in the code is likely does not
worth it.

Sergey Sharybin commented

2023-04-20 17:45:00 +02:00

@blender-bot build

Ray molenkamp commented

2023-04-20 17:48:39 +02:00

are the printf to stdout vs std::cerr << a deliberate choice?

are the `printf` to stdout vs `std::cerr <<` a deliberate choice?

Sergey Sharybin commented

2023-04-20 18:01:14 +02:00

@LazyDodo It was already inconsistent. The code is kind of just a copy-paste of a more specific catch of the corresponding try block.

There are many possible improvements, like consistent logging, RAII, code de-duplication.. I wanted to keep the change as small as possible, for the ease of cherry-pick to 3.5.1.

@LazyDodo It was already inconsistent. The code is kind of just a copy-paste of a more specific catch of the corresponding try block. There are many possible improvements, like consistent logging, RAII, code de-duplication.. I wanted to keep the change as small as possible, for the ease of cherry-pick to 3.5.1.

Sergey Sharybin commented

2023-04-20 18:29:21 +02:00

The original windows built got stuck, so poked it to -rebuild https://builder.blender.org/admin/#/builders/133/builds/929

Sergey Sharybin requested review from Brecht Van Lommel 2023-04-20 18:29:28 +02:00

Brecht Van Lommel approved these changes 2023-04-20 18:38:43 +02:00

Sergey Sharybin merged commit b2c822065c into main

2023-04-20 18:40:20 +02:00

Sergey Sharybin deleted branch fix_106977

2023-04-20 18:40:21 +02:00

Sergey Sharybin referenced this issue from a commit

2023-04-20 18:40:22 +02:00

Fix #106977: Crash when OpenEXR IO fails