Repository: telstra/open-kilda
Issue: Needs investigation: unhandled nullpointer exception in Floodlight

<details>
<summary> error </summary>

```
{
  "_index": "kilda-2020.03.10",
  "_type": "kilda-FL",
  "_id": "AXDEivoQzzwg1EoeFaPK",
  "_version": 1,
  "_score": null,
  "_source": {
    "role": "FL_ROLE_IS_UNDEFINED",
    "level": "ERROR",
    "thread": "pool-5-thread-19",
    "message": "Unhandled exception into org.openkilda.floodlight.service.session.SessionService$1: null",
    "type": "kilda-FL",
    "tags": [
      "kilda-floodlight"
    ],
    "@timestamp": "2020-03-10T13:03:27.116Z",
    "port": 59306,
    "@version": 1,
    "host": "172.22.0.15",
    "stack_trace": "java.lang.NullPointerException: null\\n\\tat org.openkilda.floodlight.service.session.Session.handleResponse(Session.java:167)\\n\\tat org.openkilda.floodlight.service.session.SwitchSessions.handleResponse(SwitchSessions.java:39)\\n\\tat org.openkilda.floodlight.service.session.SessionService.handleResponse(SessionService.java:91)\\n\\tat org.openkilda.floodlight.service.session.SessionService$1.call(SessionService.java:75)\\n\\tat org.openkilda.floodlight.service.session.SessionService$1.call(SessionService.java:72)\\n\\tat org.openkilda.floodlight.command.CommandWrapper.call(CommandWrapper.java:38)\\n\\tat org.openkilda.floodlight.command.CommandWrapper.call(CommandWrapper.java:26)\\n\\tat org.openkilda.floodlight.service.CommandProcessorService.lambda$executeOneShot$0(CommandProcessorService.java:130)\\n\\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\\n\\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\\n\\tat java.lang.Thread.run(Thread.java:748)\\n",
    "loggerName": "org.openkilda.floodlight.command.Command",
    "contextMap": {
      "correlation_id": "4d9811a1-6574-4fba-a3f3-ce8489ae51e9"
    },
    "region": "FL_REGION_IS_UNDEFINED"
  },
  "fields": {
    "@timestamp": [
      1583845407116
    ]
  },
  "highlight": {
    "level": [
      "@kibana-highlighted-field@ERROR@/kibana-highlighted-field@"
    ]
  },
  "sort": [
    1583845407116
  ]
}
```
</details>

Correlation id search shows one related debug message:
`org.openkilda.floodlight.service.of.InputService - receive message (dpId: 00:00:36:1c:7a:77:97:7e, xId: 0, version: OF_13, type: PACKET_IN)`
No related negative consequences observed

Changed files:
src-java/floodlight-service/floodlight-modules/src/main/java/org/openkilda/floodlight/service/session/Session.java
src-java/floodlight-service/floodlight-modules/src/test/java/org/openkilda/floodlight/service/session/SessionServiceTest.java

diff --git a/src-java/floodlight-service/floodlight-modules/src/main/java/org/openkilda/floodlight/service/session/Session.java b/src-java/floodlight-service/floodlight-modules/src/main/java/org/openkilda/floodlight/service/session/Session.java
index f3fca253f..41029d81d 100644
--- a/src-java/floodlight-service/floodlight-modules/src/main/java/org/openkilda/floodlight/service/session/Session.java
+++ b/src-java/floodlight-service/floodlight-modules/src/main/java/org/openkilda/floodlight/service/session/Session.java
@@ -164,7 +164,7 @@ public class Session implements AutoCloseable {
             }
 
             // check session completion (we have received all responses, if we got response for closing barrier request)
-            if (closingBarrier.isDone()) {
+            if (closingBarrier != null && closingBarrier.isDone()) {
                 incompleteRequestsStream()
                         .forEach(entry -> entry.complete(Optional.empty()));
                 return true;
@@ -181,7 +181,14 @@ public class Session implements AutoCloseable {
         CompletableFuture<Optional<OFMessage>> future = new CompletableFuture<>();
 
         long xid = message.getXid();
-        requestsByXid.put(xid, future);
+        if (xid == 0) {
+            log.error("Send OF request with xid:0 to {} - {}", sw.getId(), message);
+        }
+        CompletableFuture<Optional<OFMessage>> existing = requestsByXid.put(xid, future);
+        if (existing != null) {
+            log.error("Detect xid collision on {} xid:{} - can lead to unprocessed/missing response", sw.getId(), xid);
+            existing.complete(Optional.empty());  // to avoid infinite wait on replaced future object
+        }
         group.bindRequest(this, xid);
 
         return future;
diff --git a/src-java/floodlight-service/floodlight-modules/src/test/java/org/openkilda/floodlight/service/session/SessionServiceTest.java b/src-java/floodlight-service/floodlight-modules/src/test/java/org/openkilda/floodlight/service/session/SessionServiceTest.java
index ff43b1166..9e445c8af 100644
--- a/src-java/floodlight-service/floodlight-modules/src/test/java/org/openkilda/floodlight/service/session/SessionServiceTest.java
+++ b/src-java/floodlight-service/floodlight-modules/src/test/java/org/openkilda/floodlight/service/session/SessionServiceTest.java
@@ -39,6 +39,7 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.projectfloodlight.openflow.protocol.OFBadActionCode;
+import org.projectfloodlight.openflow.protocol.OFBadRequestCode;
 import org.projectfloodlight.openflow.protocol.OFBarrierRequest;
 import org.projectfloodlight.openflow.protocol.OFFactory;
 import org.projectfloodlight.openflow.protocol.OFMessage;
@@ -205,6 +206,38 @@ public class SessionServiceTest extends EasyMockSupport {
         }
     }
 
+    @Test
+    public void responseBeforeClose() throws Exception {
+        IOFSwitch sw = createMock(IOFSwitch.class);
+        setupSwitchMock(sw, dpId);
+        swWriteAlwaysSuccess(sw);
+        doneWithSetUp(sw);
+
+        OFFactory ofFactory = sw.getOFFactory();
+        CompletableFuture<Optional<OFMessage>> first;
+        CompletableFuture<Optional<OFMessage>> second;
+        try (Session session = subject.open(context, sw)) {
+            OFPacketOut egress = makePacketOut(ofFactory, 100);
+            first = session.write(egress);
+
+            second = session.write(makePacketOut(ofFactory, 1));
+
+            // response before close
+            OFMessage ingress = ofFactory.errorMsgs().buildBadRequestErrorMsg()
+                    .setXid(egress.getXid())
+                    .setCode(OFBadRequestCode.BAD_PORT)
+                    .build();
+            subject.handleResponse(dpId, ingress);
+        }
+
+        completeSessions(sw);
+
+        // received error response must be reported via future
+        expectExceptionResponse(first, SessionErrorResponseException.class);
+        // second request must be marker as completed
+        expectNoResponse(second);
+    }
+
     @Test
     public void sessionBarrierWriteError() throws Exception {
         IOFSwitch sw = createMock(IOFSwitch.class);
@@ -244,6 +277,12 @@ public class SessionServiceTest extends EasyMockSupport {
         }
     }
 
+    private void expectNoResponse(CompletableFuture<Optional<OFMessage>> future)
+            throws ExecutionException, InterruptedException {
+        Assert.assertTrue(future.isDone());
+        Assert.assertFalse(future.get().isPresent());
+    }
+
     private void setupSwitchMock(IOFSwitch mock, DatapathId dpId) {
         expect(mock.getId()).andStubReturn(dpId);
         expect(mock.getOFFactory()).andStubReturn(OFFactoryVer13.INSTANCE);