kwiver icon indicating copy to clipboard operation
kwiver copied to clipboard

WIP: Add mutex to temp_file_name()

Open daniel-riehm opened this issue 3 years ago • 6 comments

I encountered test failures on Windows when running ctest -j12, and traced them back to temp_file_name() giving the same temporary filename to multiple tests. This PR makes the function more thread-safe by adding a mutex, which resolved the problem for me.

daniel-riehm avatar Dec 09 '22 18:12 daniel-riehm

This is... very surprising. Unless I am missing something, why would a mutex help when the contention is across processes, which can't possibly contend on a mutex? More, why is _tempnam not returning a unique name?

mwoehlke-kitware avatar Dec 09 '22 20:12 mwoehlke-kitware

Upon further testing appears I was mistaken that this solves the problem. However, to answer @mwoehlke-kitware: the issue is that we are manually adding a suffix (e.g. .png) to the result of _tempnam. So if _tempnam returns abcdef, it is correct in that that file does not exist. But abcdef.png does exist, and that's what the function as a whole is returning once the suffix is added. Each test thread (or process?) independently gets abcdef from _tempnam, which is always the first filename this implementation of _tempnam tries, and that file never exists, because we only ever create abcdef.png. And then all the tests clobber over each other in abcdef.png. It seems adding .png, etc is necessary for indicating a particular image format to e.g. our OpenCV image writer, which takes a filename as argument. But it causes issues here.

This implementation of _tempnam (I'm using MSVC 2022) apparently just uses an increasing decimal number, instead of several pseudorandom characters like my Linux system. The temporary filename that several tests are clobbering over is test-2.tiff.

daniel-riehm avatar Dec 10 '22 00:12 daniel-riehm