Connection can't be closed when there is unread `DataFrame` in the stream
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 149
- Forks
- 59
- PR merge metrics
- No merged PRs in 30d
Description
When a stream gets a DataFrame and nothing reads data from the stream of the connection, it can't close the connection and leads to memory leak.
When I'm working on https://github.com/cri-o/cri-o memory leak issue, I found that in the situation above, spdy stream server causes memory leak.
The leak happens when it tries to close the connection, and it waits wg.Wait() forever.
The cause of the issue is that it can't finish frameHandler() because of the deadlock.
There are some workers calling frameHandler(), and wg.Add() when each worker starts and wg.Done() when each worker is done.
DataFrames queued in a PriorityFrameQueue are processed in frameHandler() and if it is a DataFrame, it is handled by dataFrameHandler() ( = handleDataFrame()).
In the function, it blocks until either the stream is closed or something reads the stream dataChan (e.g. something calls Read() or ReadData())
Otherwise the function can't finish, which means frameHandler() also can't finish, and it never calls wg.Done().
If nothing reads the data from the stream, the only way of blocking it is to close the stream.
However streams in a connection are closed after wg.Wait().
The simplest fix is to move stream.closeRemoteChannels() before wg.Wait(), but I'm unsure if it causes other race condition errors.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in connection.go at the worker handling around frameHandler(), dataFrameHandler()/handleDataFrame(), and the connection shutdown sequence around wg.Wait(). Trace how an unread DataFrame blocks on the stream data channel and how stream.closeRemoteChannels() is ordered. Done means a connection with unread stream data closes without waiting forever or leaking, while avoiding the suspected race condition.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100