Comments sent. I forgot to add this mailing list as a cc: target, though.<br><br><div class="gmail_quote">On Thu, Jun 19, 2008 at 1:17 PM, Jacob Burkhart <<a href="mailto:igotimac@gmail.com">igotimac@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">patch posted: <a href="http://codereview.appspot.com/2341" target="_blank">http://codereview.appspot.com/2341</a><br>
<br><div class="gmail_quote"><div class="Ih2E3d">On Mon, Jun 16, 2008 at 11:59 PM, Brad Fitzpatrick <<a href="mailto:brad@danga.com" target="_blank">brad@danga.com</a>> wrote:<br>
</div><div><div></div><div class="Wj3C7c"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">|| instead of 'or' (local style),<br>spaces after # in comments,<br>
wrap long lines,<br>
keep the => lined up in the constants at top,<br>use logging system instead of literal "warn"?<br><br>If you could also upload this patch once updated to <a href="http://codereview.appspot.com" target="_blank">codereview.appspot.com</a>, that'd be great. (there's a command-line tool you can download from there which does it all... no need to use the web-app)<br>
<font color="#888888">
<br>Brad</font><div><div></div><div><br><br><div class="gmail_quote">On Mon, Jun 16, 2008 at 8:19 AM, Jacob Burkhart <<a href="mailto:igotimac@gmail.com" target="_blank">igotimac@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<div><br></div><div>Can I get this patch in to djabberd?</div><div><br></div><div>Incrementing a counter on empty SSL reads is not the correct way to determine ssl_eof</div><div>(<a href="http://code.sixapart.com/trac/djabberd/changeset/758" target="_blank">http://code.sixapart.com/trac/djabberd/changeset/758</a>)</div>
<div><br></div><div>Instead we should check the error code to see if it is one of the SSL errors for which we are supposed to retry( <a href="http://www.openssl.org/docs/ssl/SSL_get_error.html" target="_blank">http://www.openssl.org/docs/ssl/SSL_get_error.html</a>)</div>
<div><br></div><div>thanks,</div><div>Jacob</div><div><div></div><div><div><br></div><div><br><br><div class="gmail_quote">On Thu, Feb 28, 2008 at 12:40 PM, Jacob Burkhart <<a href="mailto:igotimac@gmail.com" target="_blank">igotimac@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">looks like the number was original 3, and then bumped up to 10 because of observed disconnects "in practice"<div>
<a href="http://code.sixapart.com/trac/djabberd/changeset/758" target="_blank">http://code.sixapart.com/trac/djabberd/changeset/758</a><div><div></div><div><br>
<br><div class="gmail_quote">On Thu, Feb 28, 2008 at 11:33 AM, Jacob Burkhart <<a href="mailto:igotimac@gmail.com" target="_blank">igotimac@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<div><br></div><div>So we've been experiencing some bizarre behavior with clients randomly disconnecting when trying to send messages over SSL. The disconnects vary based on size of message and speed of network connections.<br>
<div><br></div><div>The problems seems to stem from:<br></div><div><a href="http://code.sixapart.com/trac/djabberd/browser/trunk/DJabberd/lib/DJabberd/Connection.pm#L494" target="_blank">http://code.sixapart.com/trac/djabberd/browser/trunk/DJabberd/lib/DJabberd/Connection.pm#L494</a><br>
</div><div><br></div><div>In <a href="http://code.sixapart.com/trac/djabberd/changeset/756" target="_blank">http://code.sixapart.com/trac/djabberd/changeset/756</a> bradfitz added code to handle non-graceful SSL disconnects. However, the number of times to retry read before giving up (10) proved to be 1 to 2 too few times in our particular situation.<br>
</div><div><br></div><div>But why did he pick 10?</div><div><br></div><div>In our tests, the number of times that Net::SSLeay::read can return zero bytes in a row varies based on the speed of a client's connection and the size of bytes attempting to be sent.</div>
<div><br></div><div>During stream initiation for instance, we experience about 2 reads of zero in a row from clients connected on reasonably fast connections, and 0 reads of zero in a row from a client running on the same machine as the server.</div>
<div><br></div><div>When sending messages larger than 8k, the number of zero reads is somewhere under 10 for clients connection over local ethernet, but is in the 11 to 12 range for clients connecting over WiFi.</div>
<div><br></div><div>So clearly, counting the number of zero bytes reads that happen is a row is not the most reliable way to determine if the client has improperly disconnected</div><div><br>
</div><div>It turns out that if we look at the Djabberd code for SSL write, we can see that there is a different answer for handling write failures.</div><div><br></div><div><a href="http://code.sixapart.com/trac/djabberd/browser/trunk/DJabberd/lib/DJabberd/Stanza/StartTLS.pm#L90" target="_blank">http://code.sixapart.com/trac/djabberd/browser/trunk/DJabberd/lib/DJabberd/Stanza/StartTLS.pm#L90</a><br>
</div><div><br></div><div>Here, get_error is called to determine if the problem might have been caused by SSL_ERROR_WANT_READ or perhaps SSL_ERROR_WANT_WRITE. If it is, then this is not a fatal condition, and we don't want to close the connection. instead, we simply return zero and expect that the writer will come around and try again at some point.</div>
<div><br></div><div>So why not do the same thing for read failures?</div><div><br></div><div>Please consider my patch (attached) as a possible solution for this problem.</div>
<div><br></div><div>thanks,</div><div>Jacob</div><div><br></div><div><br></div><div><br></div>
<div><br></div><div><br></div><div><br></div><div></div>
</div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br>
</div></div></blockquote></div></div></div><br>
</blockquote></div><br>