GIMP hang / deadlock in get_memory_table / blas_thread_shutdown

[amurzeau]

Hi,

There is a bug open in Debian related to gimp 2.10.2 and openblas 3.2: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=903514.

Depending on the machine and environment used, gimp can deadlock at startup because of a deadlock inside glibc.
I'm forwarding what I wrote for the Debian bug tracker:

Using gdb to find where it hung (gimp-gdb.txt) gives threads waiting on
a lock while doing thread-local related stuff and the main thread is in
the process of dl_close-ing openblas waiting the threads to exit using
pthread_join.

It seems that the lock used in tls_get_addr_tail [0] is the same as
the one locked by _dl_close [1].
A recursive lock is used but here it does not help as the thread calling
tls_get_addr_tail and _dl_close are not the same.

This deadlock may not happen everytime, in my case, the openblas threads
are still initializing while _dl_close is called.

Given this, I think the offending commit in openblas is bf40f80 [2]
which add TLS variables to avoid locking. But many change were done
since then.

One of related bug report is [3] which seems to indicate that the locks
handling is not easy inside glibc.

There were an attempt to fix deadlocks between tls_get_addr and a
_dl_close of a module whose finalizer joins with that thread [4].

So I see these possibles solutions:

• Add a breaks between gimp and openblas
• Disable TLS in openblas build (if possible, but this would cause a
  performance loss for users that use openblas without gimp)
• Patch glibc to not deadlock (but this seems not easy to do at all)

[0] https://github.com/bminor/glibc/blob/glibc-2.27/elf/dl-tls.c#L761
[1] https://github.com/bminor/glibc/blob/glibc-2.27/elf/dl-close.c#L812

[2]
bf40f80#diff-31f8d4e8863583d95bf2f9529f83844e
[4] https://sourceware.org/ml/libc-alpha/2015-06/msg00062.html

[amurzeau]

As I see the implementation of glibc, it seems impossible to guarantee that no deadlock will happen when dl_closing openblas.

I don't think disabling TLS in openblas is a good solution as it would cause a performance hit even for users that don't use gimp.

Have you encountered issues with TLS before ?
I don't know well openblas, but maybe there a way to ensure the threads are running and were all initialized (ie: they are waiting in their while loop in blas_thread_server) before dl_close is called by the user ?

[brada4]

Can you try building openblas with openmp? That should avoid present problem code.
There should be not that much locking in certain shutdown.

[martin-frbg]

How is Gimp built on Debian - does it use OpenMP by any chance ? A similar lockup was noted with the Arch Linux build of Gimp when using an OpenBLAS built without OpenMP support in conjuction with their OpenMP-enabled package of Gimp #240 (comment)

[brada4]

debian default is without omp, ubuntu with.
They split build into blas and lapack libs

MAKE_OPTIONS := NO_LAPACKE=1 NO_AFFINITY=1 USE_OPENMP=0 NO_WARMUP=1 CFLAGS="$(CPPFLAGS) $(CFLAGS)" FFLAGS="$(FFLAGS)" # Avoid having -O2 automatically added (especially for DEB_BUILD_OPTIONS=noopt) MAKE_OPTIONS += COMMON_OPT= FCOMMON_OPT=-frecursive

[oon3m0oo]

Would you be willing to try stripping out the logic in memory.c which sets HAS_COMPILER_TLS? I wonder if you'd have better luck with pthreads' TLS.

[amurzeau]

Thanks for your pointers :-), I will try these. -- Alexis Murzeau

[amurzeau]

I tried to compile OpenBLAS without HAS_COMPILER_TLS (with a #undef) and gimp seems to work fine over a couple of runs (10~).
No deadlock at all while with the previous binaries (with HAS_COMPILER_TLS enabled), gimp would deadlock > 50% of the time.
So pthreads' TLS seems better.

I did not try to compile OpenBLAS with open MP enabled. But I'm not sure this will change something as there is no #ifdef in memory.c about open MP (all #ifdef were replaced with USE_OPENMP_UNUSED in commit b14f44d).

I'm not sure of the implications of removing HAS_COMPILER_TLS, I would think that this is Ok to use pthreads which should have similar performance than compiler's builtins while being explicitly written in the source code.
Would this be a solution if it is confirmed that this fixes the deadlock ?

[amurzeau]

For the record, the associated Debian bug is here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=903514.

I've proposed a patch removing HAS_COMPILER_TLS to test it on others machine.

[brada4]

Can you check release before this pr?
#1625

[amurzeau]

I've tried openblas 3.2 + change memory.c contents to its content at commit f66b9c8 (this is the parent commit of the PR #1625).
No build error, and no gimp deadlock.
But I guess that this way, the performance won't be as good as just using pthread's TLS.

[oon3m0oo]

@amurzeau can you try out #1726? You'll just need to define USE_COMPILER_TLS to 0 in your build; that PR allows you to override the flag.

[oon3m0oo]

Another way to resolve this would be to call dlclose() on the right thread, which I believe is controlled by gimp. I'm also curious why gimp would be dlclosing openblas while threads are still running.

[martin-frbg]

Could be that some component of gimp is just trying to dlopen() any (optional ?) dependency on startup to see if it will be actually available when needed ? I do not think we can fix pre-existing behaviour in every caller out there, in particular if it is something that used to work before a certain change.

[brada4]

Also would be nice to check /proc/pid/maps for other blas implementation statically linked or dynamically loaded by some plugin loaded before openblas. lib(t)atlas.so being one suspect here, or mkl.