Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cgo: using cgo when passing unsafe.Pointer of a slice to C function causes memory leak #40636

Open
SnowfallDan opened this issue Aug 7, 2020 · 9 comments

Comments

@SnowfallDan
Copy link

@SnowfallDan SnowfallDan commented Aug 7, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.6 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/workspace/go/src"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build148444834=/tmp/go-build -gno-record-gcc-switches"

What did you do?

We found a memory leak when go pass a unsafe.pointer to a C function.
Code:

package main

/*
#include <stdlib.h>


int cli_upload(char *file_data, unsigned char *file_data_buf, void *etag_out)
{
	return 0;
}

*/
import "C"
import (
	"bufio"
	"fmt"
	"net/http"
	_ "net/http/pprof"
	"os"
	"runtime"
	"sync"
	"unsafe"
)

func uploadMemLeak(wg *sync.WaitGroup)  {
	buf := make([]byte, 6)
	etag_out := make([]byte, 256)

	ptr := C.CString("1006")
	defer C.free(unsafe.Pointer(ptr))

	c_err := C.cli_upload(ptr, (*C.uchar)(unsafe.Pointer(&buf)), (unsafe.Pointer(&etag_out[0])))

	fmt.Printf("upload done, err = %d\n", int(c_err))
	wg.Done()
}

func main() {
	fmt.Println(runtime.Version())
	go http.ListenAndServe("0.0.0.0:6060", nil)

	c := make(chan int)
	reader := bufio.NewReader(os.Stdin)
	_, _ = reader.ReadString('\n')

	waitGourp := sync.WaitGroup{}
	waitGourp.Add(10000)
	for i:=0; i<10000; i++ {
		go uploadMemLeak(&waitGourp)
	}
	waitGourp.Wait()
	fmt.Printf("before gc\n")
	runtime.GC()
	fmt.Printf("end gc\n")
	<-c
}

After running the code, we use pprof tool to checkout the heap memory.

we found memory leak at line 26 and 27.

What did you expect to see?

Memory recycled after C func

What did you see instead?

pprof result:

heap profile: 6: 2304 [7: 2336] @ heap/1048576
6: 2304 [6: 2304] @ 0x440ce1 0x441260 0x465b1f 0x466726 0x43b6f1
#	0x440ce0	runtime.malg+0x30		/root/sdk/go1.14.6/src/runtime/proc.go:3360
#	0x44125f	runtime.newproc1+0x44f		/root/sdk/go1.14.6/src/runtime/proc.go:3416
#	0x465b1e	runtime.newproc.func1+0x4e	/root/sdk/go1.14.6/src/runtime/proc.go:3387
#	0x466725	runtime.systemstack+0x65	/root/sdk/go1.14.6/src/runtime/asm_amd64.s:370
#	0x43b6f0	runtime.mstart+0x0		/root/sdk/go1.14.6/src/runtime/proc.go:1041

0: 0 [1: 32] @ 0x74d573 0x4687c1
#	0x74d572	main.uploadMemLeak+0x52	/root/go/src/test_mem_leak/mem_leak.go:26


# runtime.MemStats
# Alloc = 4539288
# TotalAlloc = 8506472
# Sys = 74006784
# Lookups = 0
# Mallocs = 64304
# Frees = 45619
# HeapAlloc = 4539288
# HeapSys = 63209472
# HeapIdle = 57294848
# HeapInuse = 5914624
# HeapReleased = 55083008
# HeapObjects = 18685
# Stack = 3899392 / 3899392
# MSpan = 207536 / 262144
# MCache = 13888 / 16384
# BuckHashSys = 1444202
# GCSys = 3582216
# OtherSys = 1592974
# NextGC = 9010064
# LastGC = 1596792042154702867
# PauseNs = [5824027 168789 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]
# PauseEnd = [1596792042118860538 1596792042154702867 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]
# NumGC = 2
# NumForcedGC = 1
# GCCPUFraction = 0.00010325341112560225
# DebugGC = false
@SnowfallDan SnowfallDan changed the title Using cgo pass unsafe.Pointer of a slice to C function causes memery leak Using cgo pass unsafe.Pointer of a slice to C function causes memory leak Aug 7, 2020
@SnowfallDan SnowfallDan changed the title Using cgo pass unsafe.Pointer of a slice to C function causes memory leak Using cgo when passing unsafe.Pointer of a slice to C function causes memory leak Aug 7, 2020
@randall77
Copy link
Contributor

@randall77 randall77 commented Aug 7, 2020

Can you show us exactly the commands you used to get that pprof result, so that we can reproduce it ourselves?

Maybe unrelated to this issue, but this is almost certainly not what you want:

(*C.uchar)(unsafe.Pointer(&buf))

That's a pointer to the slice header. You want

(*C.uchar)(unsafe.Pointer(&buf[0]))

That's a pointer to the first element of the slice.

@randall77 randall77 changed the title Using cgo when passing unsafe.Pointer of a slice to C function causes memory leak cmd/cgo: using cgo when passing unsafe.Pointer of a slice to C function causes memory leak Aug 7, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 7, 2020

Your wg.Done call will run before your deferred call to C.free, so it's not implausible that one buffer was left behind. See what happens if you change your code to call C.free before wg.Done.

@SnowfallDan
Copy link
Author

@SnowfallDan SnowfallDan commented Aug 10, 2020

Can you show us exactly the commands you used to get that pprof result, so that we can reproduce it ourselves?

Maybe unrelated to this issue, but this is almost certainly not what you want:

(*C.uchar)(unsafe.Pointer(&buf))

That's a pointer to the slice header. You want

(*C.uchar)(unsafe.Pointer(&buf[0]))

That's a pointer to the first element of the slice.

  1. console input:
go tool pprof http://127.0.0.1:6060/debug/pprof/heap

open browser. url is http://127.0.0.1:6060/debug/pprof/heap?debug=1

  1. we chang the code to (*C.uchar)(unsafe.Pointer(&buf[0])), result remain the same
@SnowfallDan
Copy link
Author

@SnowfallDan SnowfallDan commented Aug 17, 2020

any sugguestion?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 18, 2020

Did you see this comment? I don't see a reply.

Your wg.Done call will run before your deferred call to C.free, so it's not implausible that one buffer was left behind. See what happens if you change your code to call C.free before wg.Done.

@SnowfallDan
Copy link
Author

@SnowfallDan SnowfallDan commented Aug 19, 2020

Did you see this comment? I don't see a reply.

Your wg.Done call will run before your deferred call to C.free, so it's not implausible that one buffer was left behind. See what happens if you change your code to call C.free before wg.Done.

sorry

i delete the wg. result also remain the same....

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 19, 2020

Sorry, I wan't asking you to remove the WaitGroup. I was saying that your function uploadMemLeak does the wrong thing, because it marks the WaitGroup as done before it frees the C memory. You need to write your function like this:

func uploadMemLeak(wg *sync.WaitGroup)  {
	buf := make([]byte, 6)
	etag_out := make([]byte, 256)

	ptr := C.CString("1006")

	c_err := C.cli_upload(ptr, (*C.uchar)(unsafe.Pointer(&buf)), (unsafe.Pointer(&etag_out[0])))

	fmt.Printf("upload done, err = %d\n", int(c_err))
	C.free(unsafe.Pointer(ptr))
	wg.Done()
}

What happens if you make that change?

@SnowfallDan
Copy link
Author

@SnowfallDan SnowfallDan commented Aug 20, 2020

Sorry, I wan't asking you to remove the WaitGroup. I was saying that your function uploadMemLeak does the wrong thing, because it marks the WaitGroup as done before it frees the C memory. You need to write your function like this:

func uploadMemLeak(wg *sync.WaitGroup)  {
	buf := make([]byte, 6)
	etag_out := make([]byte, 256)

	ptr := C.CString("1006")

	c_err := C.cli_upload(ptr, (*C.uchar)(unsafe.Pointer(&buf)), (unsafe.Pointer(&etag_out[0])))

	fmt.Printf("upload done, err = %d\n", int(c_err))
	C.free(unsafe.Pointer(ptr))
	wg.Done()
}

What happens if you make that change?

Thx

I try your code in my env. Sadlly, there is still mem leak happened.

i also tried deleting the waitGourp:

package main

/*
#include <stdlib.h>


int cli_upload(char *file_data, unsigned char *file_data_buf, void *etag_out)
{
	return 0;
}

*/
import "C"
import (
	"bufio"
	"fmt"
	"net/http"
	_ "net/http/pprof"
	"os"
	"runtime"
	"unsafe"
)

func uploadMemLeak()  {
	buf := make([]byte, 6)
	etag_out := make([]byte, 256)

	ptr := C.CString("1006")

	c_err := C.cli_upload(ptr, (*C.uchar)(unsafe.Pointer(&buf[0])), (unsafe.Pointer(&etag_out[0])))

	fmt.Printf("upload done, err = %d\n", int(c_err))
	C.free(unsafe.Pointer(ptr))
}

func main() {
	fmt.Println(runtime.Version())
	go http.ListenAndServe("0.0.0.0:6060", nil)

	c := make(chan int)
	reader := bufio.NewReader(os.Stdin)
	_, _ = reader.ReadString('\n')

	for i:=0; i<10000; i++ {
		go uploadMemLeak()
	}

	reader = bufio.NewReader(os.Stdin)
	_, _ = reader.ReadString('\n')

	fmt.Printf("before gc\n")
	runtime.GC()
	fmt.Printf("end gc\n")
	<-c
}

there is still mem leak when i check the pprof heap monitor.

heap profile: 9: 2352 [13: 6992] @ heap/1048576
5: 1920 [5: 1920] @ 0x4469d1 0x446f1d 0x46f3bf 0x46ffe6 0x441771
#	0x4469d0	runtime.malg+0x30		/root/sdk/go1.14.6/src/runtime/proc.go:3360
#	0x446f1c	runtime.newproc1+0x40c		/root/sdk/go1.14.6/src/runtime/proc.go:3416
#	0x46f3be	runtime.newproc.func1+0x4e	/root/sdk/go1.14.6/src/runtime/proc.go:3387
#	0x46ffe5	runtime.systemstack+0x65	/root/sdk/go1.14.6/src/runtime/asm_amd64.s:370
#	0x441770	runtime.mstart+0x0		/root/sdk/go1.14.6/src/runtime/proc.go:1041

1: 208 [1: 208] @ 0x52e455 0x47a895 0x51eaea 0x51f03d 0x51f1fb 0x894bd8 0x472081
#	0x52e454	fmt.glob..func1+0x34		/root/sdk/go1.14.6/src/fmt/print.go:132
#	0x47a894	sync.(*Pool).Get+0xf4		/root/sdk/go1.14.6/src/sync/pool.go:148
#	0x51eae9	fmt.newPrinter+0x39		/root/sdk/go1.14.6/src/fmt/print.go:137
#	0x51f03c	fmt.Fprintf+0x3c		/root/sdk/go1.14.6/src/fmt/print.go:203
#	0x51f1fa	fmt.Printf+0xaa			/root/sdk/go1.14.6/src/fmt/print.go:213
#	0x894bd7	main.uploadMemLeak+0x1f7	/root/go/src/test_mem_leak/mem_leak.go:32

2: 192 [2: 192] @ 0x43f992 0x450d59 0x450b32 0x508e75 0x5093d3 0x50bcca 0x516b71 0x513d99 0x51f0e4 0x51f1fb 0x894bd8 0x472081
#	0x450b31	internal/poll.runtime_Semacquire+0x41	/root/sdk/go1.14.6/src/runtime/sema.go:61
#	0x508e74	internal/poll.(*fdMutex).rwlock+0x144	/root/sdk/go1.14.6/src/internal/poll/fd_mutex.go:154
#	0x5093d2	internal/poll.(*FD).writeLock+0x42	/root/sdk/go1.14.6/src/internal/poll/fd_mutex.go:239
#	0x50bcc9	internal/poll.(*FD).Write+0x59		/root/sdk/go1.14.6/src/internal/poll/fd_unix.go:255
#	0x516b70	os.(*File).write+0x80			/root/sdk/go1.14.6/src/os/file_unix.go:280
#	0x513d98	os.(*File).Write+0xe8			/root/sdk/go1.14.6/src/os/file.go:153
#	0x51f0e3	fmt.Fprintf+0xe3			/root/sdk/go1.14.6/src/fmt/print.go:205
#	0x51f1fa	fmt.Printf+0xaa				/root/sdk/go1.14.6/src/fmt/print.go:213
#	0x894bd7	main.uploadMemLeak+0x1f7		/root/go/src/test_mem_leak/mem_leak.go:32

1: 32 [1: 32] @ 0x51e649 0x525f6b 0x51f090 0x51f1fb 0x894bd8 0x472081
#	0x51e648	fmt.(*buffer).writeString+0xb8	/root/sdk/go1.14.6/src/fmt/print.go:82
#	0x525f6a	fmt.(*pp).doPrintf+0x19a	/root/sdk/go1.14.6/src/fmt/print.go:991
#	0x51f08f	fmt.Fprintf+0x8f		/root/sdk/go1.14.6/src/fmt/print.go:204
#	0x51f1fa	fmt.Printf+0xaa			/root/sdk/go1.14.6/src/fmt/print.go:213
#	0x894bd7	main.uploadMemLeak+0x1f7	/root/go/src/test_mem_leak/mem_leak.go:32

0: 0 [2: 512] @ 0x894a9a 0x472081
#	0x894a99	main.uploadMemLeak+0xb9	/root/go/src/test_mem_leak/mem_leak.go:26

0: 0 [1: 32] @ 0x894a1f 0x472081
#	0x894a1e	main.uploadMemLeak+0x3e	/root/go/src/test_mem_leak/mem_leak.go:25

0: 0 [1: 4096] @ 0x411b7c 0x413bfd 0x418a85 0x496fd1 0x44c0f9 0x44c0b4 0x44c0b4 0x44c0b4 0x43f1e4 0x472081
#	0x496fd0	unicode.init+0x1fe0	/root/sdk/go1.14.6/src/unicode/tables.go:3586
#	0x44c0f8	runtime.doInit+0xb8	/root/sdk/go1.14.6/src/runtime/proc.go:5420
#	0x44c0b3	runtime.doInit+0x73	/root/sdk/go1.14.6/src/runtime/proc.go:5415
#	0x44c0b3	runtime.doInit+0x73	/root/sdk/go1.14.6/src/runtime/proc.go:5415
#	0x44c0b3	runtime.doInit+0x73	/root/sdk/go1.14.6/src/runtime/proc.go:5415
#	0x43f1e3	runtime.main+0x183	/root/sdk/go1.14.6/src/runtime/proc.go:190


# runtime.MemStats
# Alloc = 5083328
# TotalAlloc = 8303912
# Sys = 74465536
# Lookups = 0
# Mallocs = 54574
# Frees = 29704
# HeapAlloc = 5083328
# HeapSys = 66256896
# HeapIdle = 58540032
# HeapInuse = 7716864
# HeapReleased = 49569792
# HeapObjects = 24870
# Stack = 851968 / 851968
# MSpan = 218144 / 294912
# MCache = 13888 / 16384
# BuckHashSys = 1445378
# GCSys = 3520776
# OtherSys = 2079222
# NextGC = 9836096
# LastGC = 1597903797118630653
# PauseNs = [167826 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]
# PauseEnd = [1597903797118630653 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]
# NumGC = 1
# NumForcedGC = 0
# GCCPUFraction = 0.002968485099643129
# DebugGC = false

In my code, mem leak happened at line 25 and 26.

I don't know how to fix it.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 20, 2020

If you remove the WaitGroup, it is not surprising that you see what appears to be a memory leak. You are not waiting for all the C memory to be freed before the program exits.

Can you show us the code with the corrected use of WaitGroup, and show us what you see? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.