From 764e0b7c3902cff333c45573efe4881de0cc6057 Mon Sep 17 00:00:00 2001 From: Lorand Jakab Date: Thu, 11 May 2017 13:21:24 +0300 Subject: [PATCH] Bug 8429: Fix SMR handling concurrency issue Change-Id: I1944a82600a3956c053edd9134541a6eab776e0a Signed-off-by: Lorand Jakab --- .../implementation/lisp/MapServer.java | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java index 83f0602b3..799def39c 100644 --- a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java +++ b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java @@ -356,16 +356,17 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS LOG.debug("Lazy removing expired subscriber entry " + subscriber.getString()); subscribers.remove(); } else { + final Eid srcEid = mrb.getSourceEid().getEid(); final ScheduledFuture future = executor.scheduleAtFixedRate(new CancellableRunnable( mrb, subscriber), 0L, ConfigIni.getInstance().getSmrTimeout(), TimeUnit.MILLISECONDS); final IpAddressBinary subscriberAddress = LispAddressUtil .addressBinaryFromAddress(subscriber.getSrcRloc().getAddress()); if (subscriberFutureMap.containsKey(subscriberAddress)) { - subscriberFutureMap.get(subscriberAddress).put(mrb.getSourceEid().getEid(), future); + subscriberFutureMap.get(subscriberAddress).put(srcEid, future); } else { final Map> eidFutureMap = Maps.newConcurrentMap(); - eidFutureMap.put(mrb.getSourceEid().getEid(), future); + eidFutureMap.put(srcEid, future); subscriberFutureMap.put(subscriberAddress, eidFutureMap); } } @@ -379,9 +380,10 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS if (eidFutureMap != null) { final ScheduledFuture future = eidFutureMap.get(event.getEid()); if (future != null && !future.isCancelled()) { - future.cancel(false); - LOG.debug("SMR-invoked MapRequest received, scheduled task for subscriber {} with nonce {} has " - + "been canceled", subscriberAddress.toString(), event.getNonce()); + future.cancel(true); + LOG.debug("SMR-invoked MapRequest received, scheduled task for subscriber {}, EID {} with" + + " nonce {} has been cancelled", subscriberAddress.toString(), + LispAddressStringifier.getString(event.getEid()), event.getNonce()); eidFutureMap.remove(event.getEid()); } if (eidFutureMap.isEmpty()) { @@ -406,39 +408,51 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS public void run() { final IpAddressBinary subscriberAddress = LispAddressUtil .addressBinaryFromAddress(subscriber.getSrcRloc().getAddress()); + final Eid srcEid = mrb.getSourceEid().getEid(); + try { // The address stored in the SMR's EID record is used as Source EID in the SMR-invoked // Map-Request. To ensure consistent behavior it is set to the value used to originally request // a given mapping. if (executionCount <= ConfigIni.getInstance().getSmrRetryCount()) { - mrb.setEidItem(new ArrayList()); - mrb.getEidItem().add(new EidItemBuilder().setEid(subscriber.getSrcEid()).build()); - notifyHandler.handleSMR(mrb.build(), subscriber.getSrcRloc()); - LOG.trace("{}. attempt to send SMR for MapRequest " + mrb.getSourceEid().getEid() - + " to subscriber " + subscriber.getSrcRloc(), executionCount); + synchronized (mrb) { + mrb.setEidItem(new ArrayList()); + mrb.getEidItem().add(new EidItemBuilder().setEid(subscriber.getSrcEid()).build()); + notifyHandler.handleSMR(mrb.build(), subscriber.getSrcRloc()); + if (LOG.isTraceEnabled()) { + LOG.trace("Attempt #{} to send SMR for EID {} to subscriber {}, source EID {}", + executionCount, + LispAddressStringifier.getString(mrb.getSourceEid().getEid()), + LispAddressStringifier.getString(subscriber.getSrcRloc()), + LispAddressStringifier.getString(mrb.getEidItem().get(0).getEid())); + } + } } else { LOG.trace("Cancelling execution of a SMR Map-Request after {} failed attempts.", executionCount - 1); - cancelAndRemove(subscriberAddress); + cancelAndRemove(subscriberAddress, srcEid); + return; } } catch (Exception e) { LOG.error("Errors encountered while handling SMR:", e); - cancelAndRemove(subscriberAddress); + cancelAndRemove(subscriberAddress, srcEid); + return; } executionCount++; } - private void cancelAndRemove(IpAddressBinary subscriberAddress) { + private void cancelAndRemove(IpAddressBinary subscriberAddress, Eid eid) { final Map> eidFutureMap = subscriberFutureMap.get(subscriberAddress); if (eidFutureMap == null) { LOG.warn("Couldn't find subscriber {} in SMR scheduler internal list", subscriberAddress); return; } - final Eid eid = mrb.getSourceEid().getEid(); + if (eidFutureMap.containsKey(eid)) { - eidFutureMap.get(eid).cancel(false); + ScheduledFuture eidFuture = eidFutureMap.get(eid); + eidFutureMap.remove(eid); + eidFuture.cancel(false); } - eidFutureMap.remove(eid); if (eidFutureMap.isEmpty()) { subscriberFutureMap.remove(subscriberAddress); } -- 2.36.6