Great! Would this also solve the cuda OOM?

@ssnl I don't think it's related. Since it's about shared memory but CUDA Tensors are not shared on Windows.

This PR is almost finished. However, there're some racing conditions when workers shutdown in DataLoader. Traceback listed as error type 2.

@soumith @apaszke It should be okay now. I've tested it with a very large data and saw no memory leak
or exception after running it for 10 minutes with num_worker set to 8.

@ssnl Sorry for wrong answer. I think they may be related now.

@apaszke It's my fault for not having explained it clearly. FileMapping is a kernel object on Windows. A kernel object will be released when all references to it are released. When we use FileMapping, we first create it in the child process using CreateFileMapping, and then the parent process will try to open it using OpenFileMapping. The current question is when refcount is down to zero, only one side really closes the FileMapping using CloseHandle, because when the other side tries to close it previously, the refcount of it is nonzero, so the CloseHandle is not triggered. In order to fully release the FileMapping, we need to call CloseHandle on both sides.
You may ask this question. Why don't we close it when the free function is called, since the kernel object is reserved when another reference is not closed? Because when we call CloseHandle, the handle is invalid afterwards. So we cannot use it anymore. But the multiprocessing lib of Python uses Pipe and overlapped I/O (similar to asynchronous IO), so we don't know exactly when the use of the handle is completed. So I think it's safe to close both handles when the refcount of the FileMapping decreases to zero, because this means the transfer process is done.
As for the death of the process, it can be classified into 2 types: child process dies first or parent process dies first.
For the first circumstance, it's actually safe because the parent process will be stuck (or exit on error) and the error message will be printed so that user will know there're problems and try to kill it. If parent process dies first, then the child process will die too. Because the pipe is not usable anymore, the exception will be thrown and the process will be stopped. And after both process are killed, Events are also released. (Because Event is a kernel object too.)

@apaszke The code was rewritten to use only one daemon thread for a process. I think it should be okay now.

@apaszke Please remember to review this when you are free.

@apaszke Could you review this again please?

While your changes could have addressed my inline comments in the code (haven't checked yet), I still can't see the reason why we have to add 300 lines of multithreaded code to handle a problem that seems inexistent on Windows. I still think this is not the right approach, and it could be greatly simplified.

@apaszke
I think there's still some confusion in your understanding towards the issue, so I'll explain it clearly again.

Explanation

The release for the FileMapping is described in this MSDN document.
That is to say to fully release a FileMapping, we should do the following for all the processes.

CloseHandle(handle);
UnmapViewOfFile(data);

However, currently we do it using refcount. That is

if (THAtomicDecrementRef(&info->refcount)) {
  CloseHandle(ctx->handle);
}
UnmapViewOfFile(data);

The refcount is the global count for the all the processes that hold data, not the local refcount for data inside the process. So when the global refcount is down to zero, it certainly only happen on only one process. One thing to state is that the FileMapping is used on a per tensor basis, which is not a process-level object. Thus the memory leak happens. The FileMapping is not released because the other processes still own the handle to the FileMapping.

About the simplified solution

Yes, you may argue that this implementation is not good enough. However, this issue is really existent. The memory are actually eaten up when you use multiple workers. One way to solve it more elegantly may be to delay the ctx_context_free of the shared tensor and call it when the transfer process of the data is over. But it's really hard to implement. As an alternative, we can use threadpool or sth like that. However, I don't know whether it's enough.

@peterjc123 thanks a lot for your detailed write up. This states the problem very clearly.

Still, this reassures me that I understood what you said before correctly, and I stand by my hypothesis. I agree that the current behavior (of closing only when refcount reaches zero) is incorrect! My only point is that what you wrote in this patch is unnecessary and could be greatly simplified. The reason THMapAllocator exists is exactly because the Linux kernel doesn't provide the same refcounting mechanism for shared memory objects that Windows kernel does. However, this means that using this allocator is pointless on Windows, and it could have been safely replaced with the regular THMapAllocator!

I still haven't seen any points against this in your argumentation. I'm not a Windows expert so there might still be some caveats I'm unaware of, but if this is the case then please let me know!

@apaszke Thanks for your review. Yes, we can use THMapAllocator instead of THRefMapAllocator to get rid of the refcount but actually that is not the point. There is sth that blocks the use of this, too. The multiprocessing lib of Python uses Pipe to transfer data across processes under Windows. During this process, the FileMapping must not be closed. But it uses asynchronous IO so the correct closing timing of FileMapping is not easy to get. (Insert code to multiprocessing?)
As an alternative, we can use THRefMapAllocator to confirm the IO process is actually finished when its refcount is down to zero. That is what this PR is currently doing.
Please let me know what you think.

@peterjc123 Ok, so the issue is that we're only sending the name of the shared object in this file, and this doesn't cause its refcount to increase, so we can't close it until it is opened in another process. Do I understand this correctly?

Also, is filesystem the only way of multiprocessing we support at the moment, or does file_descriptor work too?

@apaszke Yes, you are right. The problem is that we cannot get the handle(because the ctx object may be already gone) and don't know when to release it when IO completes.
I don't know exactly which one FileMapping should belong to. It uses memory instead of a hard disk. (using a file is also okay for FileMapping) The file_descriptor works on fork but the FileMapping works on spawn. The file_descriptor is shared through a number(handle) while FileMapping uses the same name to specify the same object.

@apaszke What's your idea about this?

@apaszke Done!

Thank you @peterjc123!