Bug #1054
Memory leak
100%
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.
History
Updated by Zdeněk Kopřivík over 12 years 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.
Updated by Zdeněk Kopřivík over 12 years 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.
Updated by Zdeněk Kopřivík over 12 years 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
Updated by Adam Sutton over 12 years 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.
Updated by Zdeněk Kopřivík about 12 years 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.
Updated by Adam Sutton about 12 years 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
Updated by Adam Sutton about 12 years 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
Updated by Adam Sutton about 12 years ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset commit:b2b15cb60a3d1cce31b3f9a52e322719f82b37dc.