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

runtime: memory leak was observed in tcp client go program - RSS memory keeps growing #40404

Closed
santhoshkarthi opened this issue Jul 25, 2020 · 47 comments

Comments

@santhoshkarthi
Copy link

@santhoshkarthi santhoshkarthi commented Jul 25, 2020

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

go1.14.3

Does this issue reproduce with the latest release?

Yes

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

set GO111MODULE=
set GOARCH=arm
set GOBIN=
set GOCACHE=C:\Users\irhpoq\AppData\Local\go-build
set GOENV=C:\Users\irhpoq\AppData\Roaming\go\env
set GOEXE=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=linux
set GOPATH=D:\Karthik\Skills\Golang\GolangTraining-master
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set GOARM=7
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-fPIC -marm -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\irhpoq\AppData\Local\Temp\go-build269194878=/tmp/go-build -gno-record-gcc-switches
$ go env

What did you do?

What did you expect to see?

I see RSS memory (Physical RAM) memory was keep growing in the "top" in linux

What did you see instead?

It should not grow

@santhoshkarthi santhoshkarthi changed the title Memory leak was observed in go program - RSS was keeps growing Memory leak was observed in go program - RSS memory was keep growing Jul 25, 2020
@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 25, 2020

Code :
package main

import (
	"bufio"
	"fmt"
	"net"
	"time"
)

func tcpThread() {
	fmt.Println("Entered tcp Thread...")
	conn, err := net.Dial("tcp", "127.0.0.1:8081")
	if err != nil {
		fmt.Println(err)
		return
	}
	for {
		Msg := "This is just a message"
		fmt.Fprintf(conn, Msg+"\n")
		fmt.Println("Message was sent to tcp server...")
		message, _ := bufio.NewReader(conn).ReadString('\n')
		fmt.Print("Message from tcp server: " + message)
		time.Sleep(10 * time.Second)
	}
}

func main() {
	fmt.Println("Entered into main")
	go tcpThread()
	select {}
}
@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 25, 2020

I have just written in simple TCP client and it would send message to TCP server every 10 sec

@santhoshkarthi santhoshkarthi changed the title Memory leak was observed in go program - RSS memory was keep growing Memory leak was observed in tcp client go program - RSS memory was keep growing Jul 25, 2020
@ianlancetaylor ianlancetaylor changed the title Memory leak was observed in tcp client go program - RSS memory was keep growing runtime: memory leak was observed in tcp client go program - RSS memory keeps growing Jul 25, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 25, 2020

I doubt this is related to the problem, but the statement bufio.NewReader(conn).ReadString('\n') does not work correctly. On a TCP connection this is going to read ahead and collect any data that follows the newline, and then it will throw it away. You need to call bufio.NewReader outside of the loop.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 25, 2020

What does the TCP server do?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 26, 2020

Unrelated

	go tcpThread()
	select {}

rather than starting a goroutine then going to sleep; just call tcpThread directly from main.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

@ianlancetaylor

TCP Server Code,
`package main

import (
"bufio"
"fmt"
"net"
)

// only needed below for sample processing

func main() {

fmt.Println("Launching server...")

// listen on all interfaces

ln, _ := net.Listen("tcp", ":8081")

// accept connection on port
defer ln.Close()
conn, _ := ln.Accept()

// run loop forever (or until ctrl-c)

for {

	// will listen for message to process ending in newline (\n)

	message, err := bufio.NewReader(conn).ReadString('\n')
	if err != nil {
		fmt.Println(err)
		return
	}

	// output message received

	fmt.Print("Message Received: ", string(message))

	// sample process for string received

	newmessage := "success"

	// send new string back to client

	conn.Write([]byte(newmessage + "\n"))

}

}
`

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

@santhoshkarthi thank you for the code sample.

Before we continue can you please check all error values in your sample code and address the issue @ianlancetaylor highlighted earlier, #40404 (comment)

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

@davecheney sure I will check and let you know, Thanks for your support

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

TCP_client.go

`package main

import (
"fmt"
"net"
"time"
)

func tcpThread() {
fmt.Println("Entered tcp Thread...")
conn, err := net.Dial("tcp", "127.0.0.1:8081")
if err != nil {
fmt.Println(err)
return
}
for {
Msg := "This is just a message"
fmt.Fprintf(conn, Msg+"\n")
fmt.Println("Message was sent to tcp server...")
time.Sleep(10 * time.Second)
}
}

func main() {
fmt.Println("Entered into main")
go tcpThread()
select {}
}`

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

@davecheney @ianlancetaylor I removed below line,
bufio.NewReader(conn).ReadString('\n')
Even after that my RSS memory is keep increasing

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

Do I need to flush STDOUT buffer?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

No, stdout is not buffered in Go

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

Thank you for your sample code. You called it tcp server but there is no server there, only a client which connects to 127.0.0.1:8081 and sends it a short string every 10 seconds.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

Sorry @davecheney Yeah that is tcp client code, Now i dont see frequently increasing but slowly increasing after removing below line,
"message, _ := bufio.NewReader(conn).ReadString('\n')"

but tcp_server RSS memory is increasing quickly(every 1 sec increasing 4 bytes)

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

TCP_Server code,
`package main

import (
"bufio"
"fmt"
"net"
)

// only needed below for sample processing

func main() {

fmt.Println("Launching server...")

// listen on all interfaces

ln, _ := net.Listen("tcp", ":8081")

// accept connection on port
defer ln.Close()
conn, _ := ln.Accept()

// run loop forever (or until ctrl-c)

for {

	// will listen for message to process ending in newline (\n)
	message, err := bufio.NewReader(conn).ReadString('\n')
	if err != nil {
		fmt.Println(err)
		return
	}
	// output message received

	fmt.Print("Message Received: ", string(message))

	// sample process for string received

	//newmessage := "success"

	// send new string back to client

	//conn.Write([]byte(newmessage + "\n"))

}

}
`

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

Can you please post the version of the code with the error handling changes I suggested as well as addressing this line

message, err := bufio.NewReader(conn).ReadString('\n')

Constructing a new bufio.Reader on each iteration is not correct.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

Are you asking for tcp_server or tcp_client or both?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

This code, #40404 (comment)

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

@davecheney if i move "message, err := bufio.NewReader(conn).ReadString('\n')" outside for loop, it is printing "fmt.Print("Message Received: ", string(message))" with last received message continuously

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

That is expected. The mistake is you should not construct a NewRedeader inside the loop, construct it outside the loop then call ReadString inside the loop to read a line at a time.

Or consider using something like bufio.NewScanner which is the better choice to read lines at a time.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

TCP_server code

` package main

import (
"bufio"
"fmt"
"net"
)

// only needed below for sample processing

func main() {

fmt.Println("Launching server...")

// listen on all interfaces

ln, _ := net.Listen("tcp", ":8081")

// accept connection on port
defer ln.Close()
conn, _ := ln.Accept()

// run loop forever (or until ctrl-c)
message, err := bufio.NewReader(conn).ReadString('\n')
if err != nil {
	fmt.Println(err)
	return
}

for {

	// will listen for message to process ending in newline (\n)

	// output message received

	fmt.Print("Message Received: ", string(message))

	// sample process for string received

	//newmessage := "success"

	// send new string back to client

	//conn.Write([]byte(newmessage + "\n"))

}

}
`

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

This is what you are asking me to do right @davecheney ?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

Please review #40404 (comment)

and confirm if the original problem is fixed.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

sure @davecheney I will check that, Thanks

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

@davecheney yeah now it is somewhat okay but still it is slowly increasing,
image
after 5 mins, it was increased to "1920" bytes from 1888 bytes
image

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

where as tcp_client was stable

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

New TCP server code, based on your suggestion,
`
package main

import (
"bufio"
"fmt"
"io"
"net"
)

// only needed below for sample processing

func main() {

fmt.Println("Launching server...")

// listen on all interfaces

ln, _ := net.Listen("tcp", ":8081")

// accept connection on port
defer ln.Close()
conn, _ := ln.Accept()

// run loop forever (or until ctrl-c)
message := bufio.NewReader(conn)
for {

	// will listen for message to process ending in newline (\n)

	// output message received

	msg, err := message.ReadString('\n')
	if len(msg) == 0 && err != nil {
		if err == io.EOF {
			break
		}
	}
	fmt.Print("Message Received: ", string(msg))

	// sample process for string received

	//newmessage := "success"

	// send new string back to client

	//conn.Write([]byte(newmessage + "\n"))

}

}

`

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

I believe those values are in kilobytes, so the process grew by 32 kilobytes in 5 minutes. Does it grow steadily, or are the jumps quantised? Does the growth continue if you leave the process? If you export GODEBUG=gctrace=1 does the output match the growth in memory usage as reported by top?

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

yeah it grows steadily, after I left the process ideal, now it grew up to 2164 KB from 1888KB, approx 300KB in 20 mins

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

Attaching tcp_server code for your reference,

`package main

import (
"bufio"
"fmt"
"io"
"net"
)

// only needed below for sample processing

func main() {

fmt.Println("Launching server...")

// listen on all interfaces

ln, _ := net.Listen("tcp", ":8081")

// accept connection on port
defer ln.Close()
conn, _ := ln.Accept()

// run loop forever (or until ctrl-c)
message := bufio.NewReader(conn)
for {

	// will listen for message to process ending in newline (\n)

	// output message received

	msg, err := message.ReadString('\n')
	if len(msg) == 0 && err != nil {
		if err == io.EOF {
			break
		}
	}
	fmt.Print("Message Received: ", string(msg))

	// sample process for string received

	//newmessage := "success"

	// send new string back to client

	//conn.Write([]byte(newmessage + "\n"))

}

}
`

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 27, 2020

Is this because I am creating new variable "msg, and err" everytime,
msg, err := message.ReadString('\n')
@davecheney

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 27, 2020

I don’t think so, once that variable falls out of scope it’s storage should be freed by the garbage collector. The gctrace data I asked you to collect would confirm it. Do you have that data?

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

@davecheney I am running GO program in imx6ul(ARMv7) embedded board, It does not have go compiler installed, I am just cross compiling go code in my Host PC and running it in my target. So is it possible to export "GODEBUG=gctrace=1" in the target

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

yes, those variables affect the binary at run time, https://godoc.org/runtime

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

So I just add this line into my go code, **"GODEBUG=gctrace=1"**Would that work sir?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

This is an environment variable, export it in the same shell as you run the program.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

@davecheney , yeah I have exported it in my target linux machine, where would i get the expected result/output?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

It will be printed to stderr as the program runs. Consult the documentation for more detail, https://godoc.org/runtime.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

@davecheney Program is running for around 20 mins, and RAM memory was grew up from 4536KB to 4720KB but still there is no garbage collected message printed in the console, like "gc # @#s #%: #+#+# ms clock, #+#/#/#+# ms cpu, #->#-># MB, # MB goal, # P"
Is it possible to identify why RAM memory is growing

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

It it likely that you did not correctly set the environment variable.

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

"export GODEBUG=gctrace=1" this is what i did

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

Can you show what you did? A screenshot or a copy an paste of the terminal session if possible?

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

root@ccimx6ulsbc:/home/root# export GODEBUG=gctrace=1

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

Can you should me more? What happened after you set the environment variable and ran the program?

@santhoshkarthi
Copy link
Author

@santhoshkarthi santhoshkarthi commented Jul 28, 2020

after I set the variable and ran the program nothing was printed, I think i have mentioned already that my Embedded board does not have GO compiler installed, I am just cross compiling in host machine and running the binary in my target, Would it still work?

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 28, 2020

Yes, this is supported.

To assist you we need to see more than one line. Please review #40404 (comment)

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 30, 2020

Based on the discussion in #40448 I am closing this as a duplicate.

@davecheney davecheney closed this Jul 30, 2020
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.