@amadio commented on this pull request.


In src/XrdHttp/XrdHttpProtocol.cc:

> @@ -1759,10 +1759,17 @@ void XrdHttpProtocol::Cleanup() {
 
   if (ssl) {
 
-
-    if (SSL_shutdown(ssl) != 1) {
-      TRACE(ALL, " SSL_shutdown failed");
-      ERR_print_errors(sslbio_err);
+    int ret = SSL_shutdown(ssl);
+    if (ret != 1) {
+        if(ret == 0) {
+            //https://www.openssl.org/docs/man1.0.2/man3/SSL_shutdown.html
+            //Call again SSL_shutdown
+            ret = SSL_shutdown(ssl);

OK, this will work but it does slow the connection turnaround because it tries to do a bidirectional shutdown which I don't think is needed. So, while not technically wrong there is a possible improvement and I leave it up to you to decide which way to go.

The documentation says that bidirectional shutdown is necessary if the connection is going to be reused. If the connection will be closed anyway, then the unidirectional shutdown is enough. Therefore, if we are going to reuse the connection in question here, we should do the complete shutdown, as done by @ccaffy here. However, I think from the above, before calling any shutdown function, we must check that there are no pre-existing errors on the connection.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/1968/review/1348928980@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1968#discussion_r1142370529", "url": "https://github.com/xrootd/xrootd/pull/1968#discussion_r1142370529", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1