runtime: go1.3 windows/amd64:system handle leaks in multithread c-callback

[gopherbot]

by policehsh:

run attachments,you can see each time we call test_handle_leaks_callback , a system
handle is leaked(you can Watch handles in windows task manager)

Attachments:

1. main.go (286 bytes)
2. test.c (321 bytes)
3. test.h (93 bytes)

[dvyukov]

Comment 1:

Labels changed: added release-go1.4, repo-main.

Status changed to Accepted.

[dvyukov]

Comment 2:

Alex, I think the issue is here:
// Called to initialize a new m (including the bootstrap m).
// Called on the new thread, can not allocate memory.
void
runtime·minit(void)
{
    void *thandle;
    // -1 = current process, -2 = current thread
    runtime·stdcall(runtime·DuplicateHandle, 7,
        (uintptr)-1, (uintptr)-2, (uintptr)-1, &thandle,
        (uintptr)0, (uintptr)0, (uintptr)DUPLICATE_SAME_ACCESS);
    runtime·atomicstorep(&g->m->thread, thandle);
}
// Called from dropm to undo the effect of an minit.
void
runtime·unminit(void)
{
}
We leak the thread handle during each C->Go callback on external thread.
We need to close the handle. We can't keep it for later as, I think, extram's are reused
across different threads.

[dvyukov]

Comment 3:

We also need to close per M sema used in lock_sema.c and maybe some other per-thread
handles.

[dvyukov]

Comment 4:

OK, the sema can stay there, it will be reused for future callbacks along with the
extram.

[ianlancetaylor]

Comment 5:

Labels changed: added os-windows.

[alexbrainman]

Comment 6:

I will try to fix this, but I am busy now. A test would be nice, but ...
Calling Go from C on non-Go thread does not work. See issue #6751. I think we should fix
both issues. I tried to fix issue #6751 (see diff in
https://golang.org/issue/6751?c=3). but that triggers "all
goroutines are asleep - deadlock!" panic. I could not work out what needs to be adjusted
in checkdead.
Alex

[alexbrainman]

Comment 7:

Dmitriy, linux and windows race builders are off dashboard.
Alex

[dvyukov]

Comment 8:

issue #6751 is about syscall.NewCallback, this issue is about cgo callbacks.

[gopherbot]

Comment 9:

CL https://golang.org/cl/145890044 mentions this issue.

[rsc]

Comment 10:

This issue was closed by revision 88d53dd.

Status changed to Fixed.

[gopherbot]

Comment 11:

CL https://golang.org/cl/148790043 mentions this issue.

[alexbrainman]

Comment 12:

This issue was updated by revision 9ca8368.

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/148790043