Hi,
I have a deadlock at the end of my programs when I use FFTW, shared libs and MSVC. You can see it here:
http://my.cdash.org/viewTest.php?onlyfailed&buildid=1335053.
Note that I don’t have this problem under linux:
my.cdash.org/index.php?project=RTK&date=2017-12-08
After recompiling FFTW with cmake in debug mode, I nailed it down to a deadlock in the function kill_workforce. I guess the problem can also come from FFTW but I know they don’t want to answer Windows questions. Has anyone encountered such an issue and would you have any suggestion on how to debug this? My feeling is that it comes from a destruction order issue…
Thanks in advance,
Simon
This sounds pretty much like the thread pool destructor deadlock bug from last month.
@dzenanz As I recall that bug occurred after the program exited the main, and resource were trying to be freed properly.
@simon.rit Can you get the stack trace with the program is in deadlock?
I can. I don’t have it with me but it was after in main, in fftw, and in
https://github.com/InsightSoftwareConsortium/ITK/blob/30206c0ec0cfecb6df9dec22e9a54e98463189db/Modules/Filtering/FFT/src/itkFFTWGlobalConfiguration.cxx#L552
on the ITK side. I’ll send the full stack on Monday if you need it.
Thread pool bug in short: the threads get killed by Windows before the destructor gets called, so destructor hangs waiting for non-existing threads to signal the semaphore.
The exact same thing seems to be going on here. As you are getting us the stack trace, you might as well confirm the above hypothesis. Pause the program, look in Threads (ctrl-alt-H on VisualStudio) and see if there is only one left.
Here is the stack
[External Code]
fftw3.dll!os_sem_down(void * * s) Line 206 C
fftw3.dll!kill_workforce() Line 373 C
fftw3.dll!fftw_threads_cleanup() Line 458 C
fftw3.dll!fftw_cleanup_threads() Line 65 C
ITKFFT-4.12.dll!itk::FFTWGlobalConfiguration::~FFTWGlobalConfiguration() Line 557 C++
[External Code]
ITKCommon-4.12.dll!itk::LightObject::UnRegister() Line 165 C++
ITKCommon-4.12.dll!itk::Object::UnRegister() Line 447 C++
ITKFFT-4.12.dll!itk::SmartPointeritk::FFTWGlobalConfiguration::UnRegister() Line 170 C++
ITKFFT-4.12.dll!itk::SmartPointeritk::FFTWGlobalConfiguration::~SmartPointeritk::FFTWGlobalConfiguration() Line 67 C++
[External Code]
And there is only one thread indeed. It’s not obvious how to apply a similar strategy as what has been done in the ITK patch since they do not even record the thread address… Any suggestion?
Does fftw have a method you can use to manually cleanup?
edit: correct auto-correct
In FFTW? I guess that’s the purpose of fftw_cleanup_threads().
With this additional line in threads/threads.c of FFTW3, it seems to work:
diff --git a/threads/threads.c b/threads/threads.c
index 9f40363..dca28ef 100644
--- a/threads/threads.c
+++ b/threads/threads.c
@@ -367,6 +367,7 @@ static void kill_workforce(void)
worker_queue = q->cdr;
q->w = &w;
os_sem_up(&q->ready);
+ if (WaitForSingleObject(q->ready, 0) == WAIT_TIMEOUT)
os_sem_down(&termination_semaphore);
unmake_worker(q);
}I guess I will report it to FFTW to let them find a suitable implementation when this is confirmed unless someone has a better suggestion.
1 Like
Have you tried adding a call to fftw_cleanup_threads to manually clean up the fftw threads before exiting of the main?
I just tried. Yes, it works: if I add fftw_cleanup_threads() and fftwf_cleanup_threads() before main ends, there is no deadlock. However, I’m not sure I fully understand your comment: it seems to me that this is not a solution if you don’t want to ask every ITK user who uses FFTW + shared libs + Windows to add these lines at the end of their main. Is there any other solution to let ITK do these cleanups automatically as was meant by the FFTWGlobalConfiguration destructor?
Or do you advise me to do a pull request with my patch on FFTW?
I think it is better to do a pull request to FFTW, as the problem is there. If we only fix it in ITK, the other users of FFTW might still suffer (or have to resort to static libraries).
My suggestion is a work around to enable your code to work with current ITK and current FFTW and not depend on a patch version of either. That can be a valuable solution depending on the requirements.
I agree that ideally, the user should not have to worry about this.
Thanks for the suggestion. What do you think of not calling fftw_cleanup_threads() in FFTWGlobalConfiguration in this configuration (I guess depending on ITKFFT_EXPORT)? I can prepare an ITK patch if you’d agree.
I’ll work on a PR for FFTW meanwhile.
This thread deadlocking issue appears to be a new thing. Do we know what VS version or OS, or libraries are required to reproduced it? Or has is always been there?
I don’t know (I was not handling the Windows compilation of RTK before but I guess FFTW was not activated in ITK). The bug occurs with Visual Studio 12 2013 Win64 on Windows 7 (see ctest script file). I could try to do other MSVC if this is really important.
ITK’s thread pool bug was occurring on VS2013 and VS2017. But judging by Windows official documentation, it should happen not matter which compiler you are using, much less the exact version of it.
Perhaps it has only been revealed (edit) recently because of the way ITK has changed to use shared libraries.
@simon.rit I don’t think there is much harm is remove the proper thread shutdown for FFT for the case of shared libraries (ITK_BUILD_SHARED_LIBS), and MSVC.
@simon.rit’s patch has been integrated in ITK. When Updating FFTW in ITK, one will have to verify that this problem also has been solved in FFTW, so carry over this patch in the new FFTW.
If some people can explain the problem to the FFTW community, I encourage you to participate to the conversation on FFTW github: https://github.com/FFTW/fftw3/pull/121.