Fix rvio segmentation fault when using threads to encode image sequences

[chxmberland]

Linked issues

Fixes #486

Summarize your change.

The rvio library allows the usage of multiple threads, which causes issues as some functions are not thread safe. In this PR, I've added a thread lock to prevent double-freeing a pointer (line 473 and 484) using mutex.

Describe the reason for the change.

Any user trying to use the rvio library with more than one thread caused a segmentation fault because of multiple double-frees.

The reason for this was that the rvio library did not initialize TwkFB::GenericIO, which meant that the old thread locks were not initialized. This code was written before mutex was added to std as well, so the old thread locks have been replaced with mutex, since it is now included in the std library. This saves a significant amount of code.

Describe what you have tested and on which operating system.

Tested the following command (used to replicate the bug initially) on MacOS.

./rvio -rthreads 2 clip1.mov -o out.#.jpg

Add a list of changes, and note any that might need special attention during the review.

See change summary.

If possible, provide screenshots.

Error before changes:

dlopen(v??b??, 0x0001): tried: 'v??b??' (no such file), '/System/Volumes/Preboot/Cryptexes/OSv??b??' (no such file), '/Users/<user>/Dev/OpenRV/_build/stage/app/RV.app/Contents/Frameworks/v??b??' (no such file), '/Users/<user>/Dev/OpenRV/_build/stage/app/RV.app/Contents/lib/v??b??' (no such file), '/usr/lib/v??b??' (no such file, not in dyld cache), 'v??b??' (no such file), '/usr/local/lib/v??b??' (no such file), '/usr/lib/v??b??' (no such file, not in dyld cache) WARNING: failed to load zsh: segmentation fault rvio -rthreads 2 ~/Downloads/friendly-lot.mp4 -o ./Down

[chxmberland]

Turns out, the thread lock that we were using was not working properly. We needed to switch to the mutex thread lock, which fixed all of the errors.

[bernie-laberge]

@chxmberland : I think I know why the original mutex was not working: I think it was never initialized:

!!!GenericIO::findByExtension()-before lockerGuard()-m_locker=0x0, threadID: 0x309789000
!!!GenericIO::findByExtension()-after lockerGuard()-m_locker=0x0, threadID: 0x309789000
!!!GenericIO::findByExtension()-before lockerGuard()-m_locker=0x0, threadID: 0x309b8c000
!!!GenericIO::findByExtension()-after lockerGuard()-m_locker=0x0, threadID: 0x309b8c000

And the PluginsLockerGuard() was not guarding anything with a null pointer.

Do you know why the m_locker is not initialized in this scenario ?

[bernie-laberge]

OK I think I've found the source of the issue. That's because unlike rv, rvio never calls TwkFB::GenericIO::init() and TwkMovie::GenericIO::init() which Initialize the plugins statics.

https://github.com/AcademySoftwareFoundation/OpenRV/blob/980236409479ec28120d1b70e627dfcde929b624/src/bin/apps/rv/main.cpp#L378

[bernie-laberge]

So this why your PR, by replacing the original never initialized m_locker mutex, fixes the segmentation fault issue. This is great. Plus it removes code. Less code less bugs. But to be like RV, rvio should also initialize the plugin statics here: https://github.com/AcademySoftwareFoundation/OpenRV/blob/980236409479ec28120d1b70e627dfcde929b624/src/bin/imgtools/rvio/main.cpp#L1035

TwkFB::GenericIO::init(); // Initialize TwkFB::GenericIO plugins statics
TwkMovie::GenericIO::init(); // Initialize TwkMovie::GenericIO plugins statics 

And also call the associated shutdowns before exiting.