Change in vdsm[master]: protocoldetector: Remove connection after handshake error

danken at redhat.com danken at redhat.com
Tue Oct 21 20:06:47 UTC 2014


Dan Kenigsberg has submitted this change and it was merged.

Change subject: protocoldetector: Remove connection after handshake error
......................................................................


protocoldetector: Remove connection after handshake error

Commit e414fa9ff9 added support for asynchronous SSL handshake, but did
not clean up properly after handshake error.

Normally, the closed connection would be removed later by the periodical
cleanup. However leaving closed socket registered with the poller is not
a good idea.

While verifying this fix, I found an unexpected failure mode - when
trying to connect to vdsm using an http client instead of https. For
example, this command:

    vdsClient 0 getVdsCaps

Causes a busy loop, handling write event on the *closed* socket, logging
this error:

    Detector thread::DEBUG::2014-10-10 00:13:29,709::protocoldetector::226
    ::vds.MultiProtocolAcceptor::(_process_handshake) Error during
    handshake: http request

The busy loop ends after about 90 seconds, writing 1163139 errors and
crating 175MB log.

An interesting question is why do we get write events on a closed
socket? Digging into M2Crypto source reveals that Connection.close()
does *not* close a socket, and it is actually closed in __del__. Since
we hold a reference to the connection, it is closed only when the
connection is removed from the _pending_connections map. See
https://github.com/M2Crypto/M2Crypto/blob/53d740ebd0fabf0346ebead4646b6f4a2e3ef782/M2Crypto/SSL/Connection.py#L63

This patch removes the connection if the handshake fails, and ensures
that we do not try to handle write event on a removed connection.

The event loop is handling read event and write event on pending
connections. If a connection is removed while handling a read event,
there is no need to handle write event on the same fd.  This is a common
problem with asynchronous event loops. asyncore had the same issue in
older versions.

Change-Id: Ide51d896c1e98b625fdc757042452dffd06ed9a1
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/33982
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Tested-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/protocoldetector.py
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved



-- 
To view, visit http://gerrit.ovirt.org/33982
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ide51d896c1e98b625fdc757042452dffd06ed9a1
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list