<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style>
<!--
@font-face
{font-family:PMingLiU}
@font-face
{font-family:PMingLiU}
@font-face
{font-family:Calibri}
@font-face
{font-family:PMingLiU}
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Calibri","sans-serif"}
a:link, span.MsoHyperlink
{color:blue;
text-decoration:underline}
a:visited, span.MsoHyperlinkFollowed
{color:purple;
text-decoration:underline}
span.EmailStyle17
{font-family:"Calibri","sans-serif";
color:windowtext}
.MsoChpDefault
{font-family:"Calibri","sans-serif"}
@page WordSection1
{margin:72.0pt 90.0pt 72.0pt 90.0pt}
div.WordSection1
{}
-->
</style>
</head>
<body lang="ZH-TW" link="blue" vlink="purple" style="">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US">Hi,</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">I'm using live555MediaServer on linux to stream HLS and I found problem in TCPStreamSink that may</span></p>
<p class="MsoNormal"><span lang="EN-US">cause memory leak and incomplete response.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">In TCPStreamSink::processBuffer() there is a 'if' statement 'if (envir().getErrno() != EPIPE)'
</span></p>
<p class="MsoNormal"><span lang="EN-US">that checks whether client closes connection during send() call.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">After 'EPIPE' error occurs, this 'if' statement may cause memory leak and the response to client
</span></p>
<p class="MsoNormal"><span lang="EN-US">can't be sent completely ever after, if socket send buffer is getting full during send() call(i.e.
</span></p>
<p class="MsoNormal"><span lang="EN-US">int a = send(socknum, data_ptr, b, 0), the result is a < b and a != -1).</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">I'm going to explain it:</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">In TCPStreamSink.cpp,</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">void TCPStreamSink::processBuffer() {</span></p>
<p class="MsoNormal"><span lang="EN-US">...</span></p>
<p class="MsoNormal"><span lang="EN-US"> int numBytesWritten</span></p>
<p class="MsoNormal"><span lang="EN-US"> = send(fOutputSocketNum, (const char*)&fBuffer[fUnwrittenBytesStart], numUnwrittenBytes(), 0);</span></p>
<p class="MsoNormal"><span lang="EN-US"> if (numBytesWritten < (int)numUnwrittenBytes()) {</span></p>
<p class="MsoNormal"><span lang="EN-US"> // The output socket is no longer writable. Set a handler to be called when it becomes writable again.</span></p>
<p class="MsoNormal"><span lang="EN-US"> fOutputSocketIsWritable = False;</span></p>
<p class="MsoNormal"><span lang="EN-US"> if (envir().getErrno() != EPIPE) { // on this error, the socket might still be writable, but no longer usable</span></p>
<p class="MsoNormal"><span lang="EN-US"> envir().taskScheduler().setBackgroundHandling(fOutputSocketNum, SOCKET_WRITABLE, socketWritableHandler, this);</span></p>
<p class="MsoNormal"><span lang="EN-US"> }</span></p>
<p class="MsoNormal"><span lang="EN-US"> }</span></p>
<p class="MsoNormal"><span lang="EN-US">...</span></p>
<p class="MsoNormal"><span lang="EN-US">}</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">1.memory leak:</span></p>
<p class="MsoNormal"><span lang="EN-US">If client closes connection while sending, 'numBytesWritten' will be -1 and 'errno' will be set to 'EPIPE'.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">The task will not be scheduled and 'onSourceClosure()' will has no chance to be called.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Since the socket is no longer usable(according to changelog.txt on 2013.10.25), the socket and
</span></p>
<p class="MsoNormal"><span lang="EN-US">resource for this session should be released when 'EPIPE' error occurs. (Normally, if no error occurs,
</span></p>
<p class="MsoNormal"><span lang="EN-US">afterStreaming(in RTSPClientConnectionSupportingHTTPStreaming) is responsible for this.)</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">And this results in memory leak.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">2.incomplete response:</span></p>
<p class="MsoNormal"><span lang="EN-US">The two 'if' statement in 'TCPStreamSink::processBuffer()' will make the situation that
</span></p>
<p class="MsoNormal"><span lang="EN-US">"socket send buffer becomes full during a send() call" be considered as 'EPIPE' error,
</span></p>
<p class="MsoNormal"><span lang="EN-US">if there were a 'EPIPE' error in previous session.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">In this case, 'numBytesWritten' would not be -1 and errno will not be changed.</span></p>
<p class="MsoNormal"><span lang="EN-US">Since errno is still 'EPIPE', 'if (envir().getErrno() != EPIPE)' will be false and then the task won't be scheduled.
</span></p>
<p class="MsoNormal"><span lang="EN-US">The sending task is stopped but it should actually try it later.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">And this results in incomplete response.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">I have considered a suggestion to this problem:</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Here are 2 references for errno handling,</span></p>
<p class="MsoNormal"><span lang="EN-US">1.</span></p>
<p class="MsoNormal"><span lang="EN-US">http://man7.org/linux/man-pages/man2/send.2.html</span></p>
<p class="MsoNormal"><span lang="EN-US">RETURN VALUE top</span></p>
<p class="MsoNormal"><span lang="EN-US"> On success, these calls return the number of bytes sent. On error,</span></p>
<p class="MsoNormal"><span lang="EN-US"> -1 is returned, and errno is set appropriately.</span></p>
<p class="MsoNormal"><span lang="EN-US">2.</span></p>
<p class="MsoNormal"><span lang="EN-US">https://resources.sei.cmu.edu/downloads/secure-coding/assets/sei-cert-c-coding-standard-2016-v01.pdf</span></p>
<p class="MsoNormal"><span lang="EN-US">13.1 ERR30-C. Set errno to zero before calling a library function known</span></p>
<p class="MsoNormal"><span lang="EN-US">to set errno, and check errno only after the function returns a value</span></p>
<p class="MsoNormal"><span lang="EN-US">indicating failure</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">According to man-pages, the send() will set errno when -1 is returned.</span></p>
<p class="MsoNormal"><span lang="EN-US">And if errno is going to be checked, the send() needs to return a value indicating failure,
</span></p>
<p class="MsoNormal"><span lang="EN-US">as sei-cert-c-coding-standard suggested.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">The value returned by send() should also be examined for indicating that 'EPIPE' is actually
</span></p>
<p class="MsoNormal"><span lang="EN-US">set by this send() call, not others.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">'onSourceClosure()' can also be used in this situation to release resource.</span></p>
<p class="MsoNormal"><span lang="EN-US">Or maybe a new method that can handle resource can be defined in TCPStreamSink's scope
</span></p>
<p class="MsoNormal"><span lang="EN-US">(onSourceClosure() is defined in MediaSink class which may cause a misunderstanding of
</span></p>
<p class="MsoNormal"><span lang="EN-US">the purpose of the method if using it to handling EPIPE error.)</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Setting errno to 0 may be unnecessary if return value of send() is examined to confirm 'EPIPE',
</span></p>
<p class="MsoNormal"><span lang="EN-US">but it may still be worth doing it.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">void TCPStreamSink::processBuffer() {</span></p>
<p class="MsoNormal"><span lang="EN-US">...</span></p>
<p class="MsoNormal"><span lang="EN-US"> errno = 0;</span></p>
<p class="MsoNormal"><span lang="EN-US"> int numBytesWritten</span></p>
<p class="MsoNormal"><span lang="EN-US"> = send(fOutputSocketNum, (const char*)&fBuffer[fUnwrittenBytesStart], numUnwrittenBytes(), 0);</span></p>
<p class="MsoNormal"><span lang="EN-US"> if (numBytesWritten < (int)numUnwrittenBytes()) {</span></p>
<p class="MsoNormal"><span lang="EN-US"> // The output socket is no longer writable. Set a handler to be called when it becomes writable again.</span></p>
<p class="MsoNormal"><span lang="EN-US"> fOutputSocketIsWritable = False;</span></p>
<p class="MsoNormal"><span lang="EN-US"> if (!(numBytesWritten == -1 && envir().getErrno() == EPIPE)) { // on this error, the socket might still be writable, but no longer usable</span></p>
<p class="MsoNormal"><span lang="EN-US"> envir().taskScheduler().setBackgroundHandling(fOutputSocketNum, SOCKET_WRITABLE, socketWritableHandler, this);</span></p>
<p class="MsoNormal"><span lang="EN-US"> }</span></p>
<p class="MsoNormal"><span lang="EN-US"> else{</span></p>
<p class="MsoNormal"><span lang="EN-US"> onSourceClosure();</span></p>
<p class="MsoNormal"><span lang="EN-US"> }</span></p>
<p class="MsoNormal"><span lang="EN-US"> } </span></p>
<p class="MsoNormal"><span lang="EN-US">...</span></p>
<p class="MsoNormal"><span lang="EN-US">}</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Or maybe a callback function could be added to TCPStreamSink(like fOnSendErrorFunc in
</span></p>
<p class="MsoNormal"><span lang="EN-US">MultiFramedRTPSink) for notifying user that there is a 'EPIPE' error and leave the job
</span></p>
<p class="MsoNormal"><span lang="EN-US">to release resource to be user-defined.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span style="font-family:"新細明體","serif"">…</span><span lang="EN-US"></span></p>
<p class="MsoNormal"><span lang="EN-US">if (!(numBytesWritten == -1 && envir().getErrno() == EPIPE)) {</span></p>
<p class="MsoNormal"><span lang="EN-US"> envir().taskScheduler().setBackgroundHandling(fOutputSocketNum, SOCKET_WRITABLE, socketWritableHandler, this);</span></p>
<p class="MsoNormal"><span lang="EN-US">}</span></p>
<p class="MsoNormal"><span lang="EN-US">else{</span></p>
<p class="MsoNormal"><span lang="EN-US"> if (fOnEPIPESendErrorFunc != NULL) (*fOnEPIPESendErrorFunc)(fOnEPIPESendErrorData);</span></p>
<p class="MsoNormal"><span lang="EN-US">}</span></p>
<p class="MsoNormal"><span style="font-family:"新細明體","serif"">…</span><span lang="EN-US"></span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">I'm wondering if there are some suggestions for handling this problem?
</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Thanks,</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Bruce Liang</span></p>
<p class="MsoNormal"><span lang="EN-US">Ability Enterprise Co., Ltd</span></p>
</div>
*** 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.
</body>
</html>