Uploaded image for project: 'netconf'
  1. netconf
  2. NETCONF-625

Race condition causes multiple notification subscriptions (diagnosed)

XMLWordPrintable

    • Icon: Story Story
    • Resolution: Unresolved
    • Icon: High High
    • None
    • None
    • netconf, restconf-nb
    • None

      I'm filing this on behalf of a developer on my team (Lukas), so I may not disambiguate all of these details...

      We hit a race condition in subscribing to notifications in Carbon. I have not had time to figure out if the problem exists in later versions of ODL. The errant behavior is that a subscriber can end up with multiple subscriptions to the same thing, such that a single notification causes multiple callbacks.

      The problem occurs when subscribing more than once successively in a short enough time window. 

      POST:
      http://controller-ip:8181/restconf/operations/sal-remote:create-notification-stream

      {   "input":    

      {    "notifications": [       "([http://verizon.com/yang/notification?revision=2018-11-15)event-notification]",       "([http://verizon.com/yang/notification?revision=2018-11-15)node-status-notification]"     ],     "notification-output-type": "JSON"   }

      }

      GET:
      http://controller-ip:8181/restconf/streams/stream/create-notification-stream/bnc-notification:node-status-notification,bnc-notification:event-notification

      The Carbon version of the code is here

      POST:
      https://github.com/opendaylight/netconf/blob/stable/carbon/restconf/sal-rest-connector/src/main/java/org/opendaylight/restconf/restful/services/impl/RestconfInvokeOperationsServiceImpl.java
      GET:
      https://github.com/opendaylight/netconf/blob/stable/carbon/restconf/sal-rest-connector/src/main/java/org/opendaylight/restconf/restful/services/impl/RestconfStreamsSubscriptionServiceImpl.java

      The master branch version of the code is here...

      POST:
      https://github.com/opendaylight/netconf/blob/master/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImpl.java
      GET:
      https://github.com/opendaylight/netconf/blob/master/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java

      Looking at the code, the culprit is likely in 

      ./restconf/sal-rest-connector/src/main/java/org/opendaylight/restconf/restful/utils/SubscribeToStreamUtil.java:notifYangStream
      ./restconf/sal-rest-connector/src/main/java/org/opendaylight/restconf/restful/utils/SubscribeToStreamUtil.java:registerToListenNotification
      org.opendaylight.controller.md.sal.dom.broker.impl.DOMNotificationRouter.registerNotificationListener

      probably do synchronized inside of this:

          private static void registerToListenNotification(final NotificationListenerAdapter listener,
                  final NotificationServiceHandler notificationServiceHandler) {
              if (listener.isListening()) { --->>>condition through which pass both threads in same time
                  return;                       --->>>synchronized to listener is probably ideal because it is same for both calls
              }
      
              final SchemaPath path = listener.getSchemaPath();
              final ListenerRegistration<DOMNotificationListener> registration =
                      notificationServiceHandler.get().registerNotificationListener(listener, path);
      
              listener.setRegistration(registration);
      
          }
      

      The master branch shows similar logic so will suffer the same behavior.

      The problem likely exists in NETCONF, where there is duplicate code...

      ./restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java

      There is a method with the same name and body registerToListenNotification

            SowmiyaChidambaram Sowmiya Chidambaram
            allanclarke Allan Clarke
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: