Project

General

Profile

Bug #1054

Memory leak

Added by Zdeněk Kopřivík over 12 years ago. Updated about 12 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
Category:
Streaming
Target version:
Start date:
2012-07-10
Due date:
% Done:

100%

Estimated time:
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.

History

#1

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.

#2

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.

#3

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

#4

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.

#5

Updated by Zdeněk Kopřivík over 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.

#6

Updated by Adam Sutton over 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

#7

Updated by John Törnblom over 12 years ago

  • Assignee set to John Törnblom
#8

Updated by Adam Sutton over 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

#9

Updated by Adam Sutton over 12 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset commit:b2b15cb60a3d1cce31b3f9a52e322719f82b37dc.

#10

Updated by Adam Sutton about 12 years ago

  • Target version set to 3.2

Also available in: Atom PDF