Memory leak on Cygwin when forking

[embray]

In #1450 I made a fix on Cygwin to ensure that the thread pool is properly shut down before forking process.

However, I've observed now that there's a memory leak associated with this somehow: Repeatedly shutting down the thread pool, then starting it up again to do some multi-threaded operation, then forking again and so on quickly leaks memory. I was able to reproduce this simply using Python with a Numpy linked against OpenBLAS (where np.linalg.inv happens to use dgesv):

>>> import numpy as np
>>> import subprocess as sp
>>> mat = np.random.random(9).reshape((3, 3))
>>> while True:
...     _ = np.linalg.inv(mat)
...     _ = sp.call(['true'])
...

While letting this loop run the memory usage of the original process quickly blows up to several gigabytes.

I will investigate further.

[embray]

Additionally, I do not see the same behavior on Linux.

[brada4]

It is called fork bomb....
Does same happen with python.org product?

[embray]

It's not a question of fork bombs. E.g. if I put a sleep in the loop it still leaks in the original process.

I don't believe plain-Windows Python is relevant here since it doesn't have any notion of fork and basically never shuts down the thread pool except when unloading the DLL, but it would be interesting to check.

[embray]

One thing I noticed is that CloseHandle is never called on the threads, so there is a leak in thread handles. However, fixing that doesn't fix the overall problem.

[embray]

Yeah, each time through the loop there's a memory allocation of just a smidge over 32 MB, which looks to be the same as

/* The number of bytes that will be allocated for each buffer. When allocating
   memory, we store an alloc_t followed by the actual buffer memory. This means
   that each allocation always has its associated alloc_t, without the need
   for an auxiliary tracking structure. */
static const int allocation_block_size = BUFFER_SIZE + sizeof(struct alloc_t);

from driver/others/memory.c, so some allocation isn't been freed. Just need to nail down which. It's not immediately obvious, and I wouldn't necessarily rule out a bug in Cygwin either.

[embray]

I haven't had a chance to test yet, but I think it is possibly as a consequence of compiling with the USE_TLS=1 feature.

In memory.c there is a DllMain defined that cleans up the per-thread memory allocation structures upon DLL_THREAD_DETACH. This DllMain is not being used; instead it is using the simpler one from exports/dllinit.c. To be honest it's not clear to me how the DllMain in memory.c is intended to replace it. There are some mysterious MSVC assembler hocus pocus that is opaque to me at initial glance.

In any case this does seem likely important.

[embray]

After tinkering a bit with adding per-thread memory cleanup to the actual DllMain used on Cygwin I was able to eliminate the memory leak completely. I want to clean things up and test a bit more, but I'll propose a PR a little later once I'm satisfied.

[embray]

Posted a PR in #2064. I think it's a bit hacky but it solves the problem.

[martin-frbg]

I suspect the reason for the apparent duplication is simply that the (original GotoBLAS) DllMain from exports/dllinit.c is not used by cmake builds (and accordingly MSVC).

[embray]

@martin-frbg I see; thank you for clarifying that. That makes sense, and I see that now (I've never even looked at the CMake build machinery). It might be good to eliminate this duplication, but that's something I could deal with separately.

[martin-frbg]

Not sure if elimination would be a good idea (with all the build system fixups it would entail), perhaps a comment added to the memory.c one would be sufficient.

[embray]

Should be fixed by #2064.