Memory Leak at Websocket Transport

[fletcherist]

What happened?

There is a memory/goroutine leak at websocket transport

I think this goroutine never closes after client closes was connection. https://github.com/99designs/gqlgen/blob/master/graphql/handler/transport/websocket.go#L374

responses, ctx := c.exec.DispatchOperation(ctx, rc)
for {
       // this never closes even if ctx is done
	response := responses(ctx)
	if response == nil {
		break
	}

	c.sendResponse(msg.id, response)
}

Minimal graphql.schema and models to reproduce

I created a reproduction demo https://github.com/fletcherist/gqlgen-memory-leak

versions

• go run github.com/99designs/gqlgen version
• go version go1.17.7 darwin/amd64

Here's some stats

I'm working on fix

[fletcherist]

#2168

i made a fix, please someone look into it

this fix reduces goroutine leakage in half and memory leakage

count: 5889	goroutines num: 5916	Alloc = 7 MiB	TotalAlloc = 369 MiB	Sys = 35 MiB	NumGC = 157
count: 5908	goroutines num: 5935	Alloc = 9 MiB	TotalAlloc = 370 MiB	Sys = 35 MiB	NumGC = 157
count: 5927	goroutines num: 5954	Alloc = 5 MiB	TotalAlloc = 371 MiB	Sys = 35 MiB	NumGC = 158
count: 5946	goroutines num: 5973	Alloc = 7 MiB	TotalAlloc = 372 MiB	Sys = 35 MiB	NumGC = 158
count: 5965	goroutines num: 5991	Alloc = 8 MiB	TotalAlloc = 374 MiB	Sys = 35 MiB	NumGC = 158
count: 5984	goroutines num: 6011	Alloc = 9 MiB	TotalAlloc = 375 MiB	Sys = 35 MiB	NumGC = 158

[fletcherist]

however i continue investigating the issue, because there's another goroutine leak somewhere

[StevenACoffman]

Hey, I really appreciate your persistence on this. Thanks and keep the contributions coming!

[fletcherist]

thanks for merging this, i've deployed my production env with master branch's fix, and will monitor, I hope it should do

memory without fix:

I'll edit this comment with updated data after fix

I continue to investigate the issue:)

[frederikhors]

@fletcherist your great PR has been released in 0.17.6! 😄

Do you have more insights for us?

[fletcherist]

Update: MoofMonkey found out the root cause of the problem. It turns out that in the chat demo we don't close that channel on ctx done and it causes goroutines leak. I thought it was handled internally, but it's not. However #2168 added this feature and now it works fine

[fletcherist]

I suggest reverting my PR and merging #2209 (it's already reverted here)

@frederikhors @StevenACoffman what do you think? I'm very sorry for my bad PR

[StevenACoffman]

I would like to wait until that PR fixes the tests in it, but then I can merge it and cut a new release.