Bug #1054
Memory leak
| Status: | Fixed | Start date: | 07/10/2012 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: | % Done: | 100% | ||
| Category: | Streaming | |||
| Target version: | 3.2 | |||
| Found in version: | git 9b71601b0686c9c998f6c503e1759d573c4efe64 | Affected Versions: |
Description
Sometimes while switching channels it happens that the HTTP subscription to the channel is not closed properly (connection is closed but there's no "unsubscription" message in the log). In this case the streaming thread continues to stream to some buffer which still grows and after some time (about 30 minutes) eats all memory (1 GB in my case).
I'm using EricV's branch of TVheadend with TS streaming but this doesn't have to be specific for TS streaming.
Associated revisions
Add write timeout of 30s to all TCP server operations, this is further reduced to 5s for streaming output. Fixes #1054.
History
#1
Updated by Zdeněk Kopřivík 10 months ago
Steps to reproduce this bug:
1. Connect a client to TVheadend over network (HTTP TS streaming in my case).
2. Unplug network cable from the client or shut the client down (unplug power cable).
3. Watch TVheadend consuming more and more memory.
This is actually not a true memory leak issue. The problem is that the connection never timeouts and that the buffer size is not limited so if any client dies it kills the server.
#2
Updated by Zdeněk Kopřivík 10 months ago
I've just noticed that according to "netstat -t" the connection stays in ESTABLISHED state even more than half an hour after the client has died so maybe this is just a problem in socket options.
#3
Updated by Zdeněk Kopřivík 10 months ago
The problem is that the streaming thread hangs in the write() call until the tcp socket dies (takes about 2 hours in my case). The most simple solution seems to be to set write timeout on the socket using SO_SNDTIMEO option:
file src/tcp.c, function tcp_server_start():
+ struct timeval timeout;
...
+ timeout.tv_sec = 10;
+ timeout.tv_usec = 0;
+ setsockopt(tsl->fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout));
This solves the problem for me. Please check the code (maybe change the timeout to 60s?) and merge with upstream.
Thank You.
Zdenek
#4
Updated by Adam Sutton 10 months ago
- Status changed from New to Invalid
This appears to be a reference to a user fork, please take this up with the user in question. TS code is not (currently) supported in tvheadend master repo.
#5
Updated by Zdeněk Kopřivík 9 months ago
The proposed change is not related to TS streaming but to any kind of streaming (and actually to any kind of data sending from the webui) and therefore should be accepted to main tree. It just sets timeout for write() operation on tcp socket and prevents possible Tvheadend crashes/leaking caused by the crashed/disconnected clients.
#6
Updated by Adam Sutton 9 months ago
- Category set to Streaming
- Status changed from Invalid to New
- Priority changed from High to Normal
Hi,
Sorry I'm being fairly ruthless with closing old issue and since this report specifically mentioned TS recording a referenced an unsupported (user) branch, I simply closed it without further review.
I will take another look at this and see whether something needs to be done in current code base. In the mean time if you want to provide the above patch as a pull request on github that would be much appreciated.
Adam
#7
Updated by John Törnblom 9 months ago
- Assignee set to John Törnblom
#8
Updated by Adam Sutton 9 months ago
- Assignee changed from John Törnblom to Adam Sutton
OK,
I've finally bothered to read the report
and I completely agree. It's not acceptable that the write() call blocks and can result in the streaming data being buffered indefinitely (until RAM runs out or you get lucky and things unstick).
I agree that the simple solution is to add a sensible timeout on the write() calls to the socket. I'll add this.
Adam
#9
Updated by Adam Sutton 9 months ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset b2b15cb60a3d1cce31b3f9a52e322719f82b37dc.
#10
Updated by Adam Sutton 7 months ago
- Target version set to 3.2