Uploaded image for project: 'OpenFlowPlugin'
  1. OpenFlowPlugin
  2. OPNFLWPLUG-318

ConnectionConductorImpl and HandshakeManagerImpl leak threads on exception paths

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved
    • Resolution: Done
    • None
    • None
    • General
    • None
    • Operating System: All
      Platform: All

    • 2394

    Description

      I'm running ODL on the latest stable/helium branches.

      I have a single controller and a single switch. The OpenFlow connection between the switch and the controller is OF13. The connection between the switch and controller has been bouncing frequently.

      This AM, I found my switch with 571 OFHandshake threads. 571 threads for a SINGLE connection.

      I checked by switch and controller. There was one connection between the two, but 571 threads all in a waiting state

      Upon code inspection, I believe I've found code paths where ConnectionConductorImpl will leak instances of ThreadPoolLoggingExecutor. Here's what I believe is happening:

      1- The switch initiates an OF connection to the controller
      This causes the following calls
      initChannel method of TcpChannelInitializer
      onSwitchConnected method of org.opendaylight.openflowplugin.openflow.md.core.SwitchConnectionHandlerImpl
      ConnectionConductorFactory.createConductor(...)
      ConnectionConductor connectionConductor = new ConnectionConductorImpl(connectionAdapter);
      This CTOR registers itself as the HandshakeListener
      ...
      connectionConductor.init();
      This call creates an instance of ThreadPoolLoggingExecutor to be created

      If there is an Exception in initChannel, the catch blocks close the channel, but the do not call any methods that will cause the ThreadPoolLoggingExecutor to shutdown

      2- The newly create thread from ThreadPoolLoggingExecutor calls the shake() method of ConnectionConductorImpl
      In the ConnectionConductorImpl, the calls to
      hsPool.shutdown();
      hsPool.purge();
      only happen when postHandshakeBasic is called
      postHandshakeBasic is called by onHandshakeSuccessfull

      HOWEVER in HandshakeManager.shake() there are code paths where the 'onHandshakeSuccessfull' method is never called (the catch block around line 116 of HandshakeManager. The exception path DOES close the connection, but never shuts down the ThreadPoolLoggingExecutor

      PROPOSAL:

      I believe the HandshakeListener needs a second method:
      onHandshakeFailure()
      The implementors of this interface can then clean up any resources they've allocated.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            mbobak@cisco.com Martin Bobak
            jimw@a-bb.net Jim West
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: