Hi,
I'm using live555MediaServer on linux to stream HLS and I found problem in
TCPStreamSink that may
cause memory leak and incomplete response.
In TCPStreamSink::processBuffer() there is a 'if' statement 'if
(envir().getErrno() != EPIPE)'
that checks whether client closes connection during send() call.
After 'EPIPE' error occurs, this 'if' statement may cause memory leak and the
response to client
can't be sent completely ever after, if socket send buffer is getting full
during send() call(i.e.
int a = send(socknum, data_ptr, b, 0), the result is a < b and a != -1).
I'm going to explain it:
In TCPStreamSink.cpp,
void TCPStreamSink::processBuffer() {
...
int numBytesWritten
= send(fOutputSocketNum, (const char*)&fBuffer[fUnwrittenBytesStart],
numUnwrittenBytes(), 0);
if (numBytesWritten < (int)numUnwrittenBytes()) {
// The output socket is no longer writable. Set a handler to be called
when it becomes writable again.
fOutputSocketIsWritable = False;
if (envir().getErrno() != EPIPE) { // on this error, the socket might
still be writable, but no longer usable
envir().taskScheduler().setBackgroundHandling(fOutputSocketNum,
SOCKET_WRITABLE, socketWritableHandler, this);
}
}
...
}
1.memory leak:
If client closes connection while sending, 'numBytesWritten' will be -1 and
'errno' will be set to 'EPIPE'.
The task will not be scheduled and 'onSourceClosure()' will has no chance to be
called.
Since the socket is no longer usable(according to changelog.txt on 2013.10.25),
the socket and
resource for this session should be released when 'EPIPE' error occurs.
(Normally, if no error occurs,
afterStreaming(in RTSPClientConnectionSupportingHTTPStreaming) is responsible
for this.)
And this results in memory leak.
2.incomplete response:
The two 'if' statement in 'TCPStreamSink::processBuffer()' will make the
situation that
"socket send buffer becomes full during a send() call" be considered as 'EPIPE'
error,
if there were a 'EPIPE' error in previous session.
In this case, 'numBytesWritten' would not be -1 and errno will not be changed.
Since errno is still 'EPIPE', 'if (envir().getErrno() != EPIPE)' will be false
and then the task won't be scheduled.
The sending task is stopped but it should actually try it later.
And this results in incomplete response.
I have considered a suggestion to this problem:
Here are 2 references for errno handling,
1.
http://man7.org/linux/man-pages/man2/send.2.html
RETURN VALUE top
On success, these calls return the number of bytes sent. On error,
-1 is returned, and errno is set appropriately.
2.
https://resources.sei.cmu.edu/downloads/secure-coding/assets/sei-cert-c-coding-standard-2016-v01.pdf
13.1 ERR30-C. Set errno to zero before calling a library function known
to set errno, and check errno only after the function returns a value
indicating failure
According to man-pages, the send() will set errno when -1 is returned.
And if errno is going to be checked, the send() needs to return a value
indicating failure,
as sei-cert-c-coding-standard suggested.
The value returned by send() should also be examined for indicating that
'EPIPE' is actually
set by this send() call, not others.
'onSourceClosure()' can also be used in this situation to release resource.
Or maybe a new method that can handle resource can be defined in
TCPStreamSink's scope
(onSourceClosure() is defined in MediaSink class which may cause a
misunderstanding of
the purpose of the method if using it to handling EPIPE error.)
Setting errno to 0 may be unnecessary if return value of send() is examined to
confirm 'EPIPE',
but it may still be worth doing it.
void TCPStreamSink::processBuffer() {
...
errno = 0;
int numBytesWritten
= send(fOutputSocketNum, (const char*)&fBuffer[fUnwrittenBytesStart],
numUnwrittenBytes(), 0);
if (numBytesWritten < (int)numUnwrittenBytes()) {
// The output socket is no longer writable. Set a handler to be called
when it becomes writable again.
fOutputSocketIsWritable = False;
if (!(numBytesWritten == -1 && envir().getErrno() == EPIPE)) { // on this
error, the socket might still be writable, but no longer usable
envir().taskScheduler().setBackgroundHandling(fOutputSocketNum,
SOCKET_WRITABLE, socketWritableHandler, this);
}
else{
onSourceClosure();
}
}
...
}
Or maybe a callback function could be added to TCPStreamSink(like
fOnSendErrorFunc in
MultiFramedRTPSink) for notifying user that there is a 'EPIPE' error and leave
the job
to release resource to be user-defined.
...
if (!(numBytesWritten == -1 && envir().getErrno() == EPIPE)) {
envir().taskScheduler().setBackgroundHandling(fOutputSocketNum,
SOCKET_WRITABLE, socketWritableHandler, this);
}
else{
if (fOnEPIPESendErrorFunc != NULL)
(*fOnEPIPESendErrorFunc)(fOnEPIPESendErrorData);
}
...
I'm wondering if there are some suggestions for handling this problem?
Thanks,
Bruce Liang
Ability Enterprise Co., Ltd
*** Confidentiality Note *** This e-mail message and any accompanying documents
are for sole use of the intended recipient(s) and may contain confidential and
privileged information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message.
_______________________________________________
live-devel mailing list
[email protected]
http://lists.live555.com/mailman/listinfo/live-devel
