From ac2ebcb16be3c1c17136cb479ba85f1c15dfa02f Mon Sep 17 00:00:00 2001
From: "Claudio D. Gasparini" <claudio.gasparini@pantheon.tech>
Date: Thu, 22 Mar 2018 12:24:10 +0100
Subject: [PATCH] BGPCEP-770: Lock file before processing

In this way we ensure is not still been modified
when event is raised.

Change-Id: I69047f244e78bdf4536d6f2a83d3b80c45191e90
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>
---
 .../initial/network-topology-config.xml       |  3 +-
 .../config/loader/impl/ConfigLoaderImpl.java  | 61 +++++++++++++++----
 .../loader/impl/ConfigLoaderImplTest.java     |  5 +-
 3 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/bgp/config-example/src/main/resources/initial/network-topology-config.xml b/bgp/config-example/src/main/resources/initial/network-topology-config.xml
index 3d24d09493..0d107535d0 100644
--- a/bgp/config-example/src/main/resources/initial/network-topology-config.xml
+++ b/bgp/config-example/src/main/resources/initial/network-topology-config.xml
@@ -1,4 +1,5 @@
-<?xml version="1.0" encoding="UTF-8"?><!--
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
   ~ Copyright (c) 2016 Cisco Systems, Inc. and others.  All rights reserved.
   ~
   ~ This program and the accompanying materials are made available under the
diff --git a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
index 9199d78b95..89882658b4 100644
--- a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
+++ b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImpl.java
@@ -10,18 +10,25 @@ package org.opendaylight.bgpcep.config.loader.impl;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.base.Stopwatch;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.RandomAccessFile;
 import java.net.URISyntaxException;
+import java.nio.channels.FileChannel;
+import java.nio.channels.FileLock;
 import java.nio.file.ClosedWatchServiceException;
-import java.nio.file.WatchEvent;
 import java.nio.file.WatchKey;
 import java.nio.file.WatchService;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 import javax.annotation.concurrent.GuardedBy;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.stream.XMLInputFactory;
@@ -48,12 +55,15 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
     private static final String INTERRUPTED = "InterruptedException";
     private static final String EXTENSION = "-.*\\.xml";
     private static final String INITIAL = "^";
+    private static final String READ = "rw";
+    private static final long TIMEOUT_SECONDS = 5;
     @GuardedBy("this")
     private final Map<String, ConfigFileProcessor> configServices = new HashMap<>();
     private final SchemaContext schemaContext;
     private final BindingNormalizedNodeSerializer bindingSerializer;
     private final String path;
     private final Thread watcherThread;
+    private final File file;
     @GuardedBy("this")
     private boolean closed = false;
 
@@ -62,6 +72,7 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
         this.schemaContext = requireNonNull(schemaContext);
         this.bindingSerializer = requireNonNull(bindingSerializer);
         this.path = requireNonNull(fileWatcher.getPathFile());
+        this.file = new File(this.path);
         this.watcherThread = new Thread(new ConfigLoaderImplRunnable(requireNonNull(fileWatcher.getWatchService())));
     }
 
@@ -87,7 +98,27 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
         final NormalizedNodeResult result = new NormalizedNodeResult();
         final NormalizedNodeStreamWriter streamWriter = ImmutableNormalizedNodeStreamWriter.from(result);
 
-        try (InputStream resourceAsStream = new FileInputStream(new File(this.path, filename))) {
+        final File newFile = new File(this.path, filename);
+        FileChannel channel = new RandomAccessFile(newFile, READ).getChannel();
+
+        FileLock lock = null;
+        final Stopwatch stopwatch = Stopwatch.createStarted();
+        while (lock == null || stopwatch.elapsed(TimeUnit.SECONDS) > TIMEOUT_SECONDS) {
+            try {
+                lock = channel.tryLock();
+            } catch (final IllegalStateException e) {
+                //Ignore
+            }
+            if (lock == null) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    LOG.warn("Failed to lock xml", e);
+                }
+            }
+        }
+
+        try (InputStream resourceAsStream = new FileInputStream(newFile)) {
             final XMLInputFactory factory = XMLInputFactory.newInstance();
             final XMLStreamReader reader = factory.createXMLStreamReader(resourceAsStream);
 
@@ -100,6 +131,7 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
                 LOG.warn("Failed to parse xml", e);
             } finally {
                 reader.close();
+                channel.close();
             }
         }
 
@@ -111,18 +143,21 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
         final String pattern = INITIAL + config.getSchemaPath().getLastComponent().getLocalName() + EXTENSION;
         this.configServices.put(pattern, config);
 
-        final File[] fList = new File(this.path).listFiles();
+        final File[] fList = this.file.listFiles();
+        final List<String> newFiles = new ArrayList<>();
         if (fList != null) {
-            for (final File file : fList) {
-                if (file.isFile()) {
-                    final String filename = file.getName();
+            for (final File newFile : fList) {
+                if (newFile.isFile()) {
+                    final String filename = newFile.getName();
                     if (Pattern.matches(pattern, filename)) {
-                        handleConfigFile(config, filename);
+                        newFiles.add(filename);
                     }
                 }
             }
         }
-
+        for (final String filename : newFiles) {
+            handleConfigFile(config, filename);
+        }
         return new AbstractRegistration() {
             @Override
             protected void removeRegistration() {
@@ -140,7 +175,7 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
 
 
     @Override
-    public synchronized void close() throws Exception {
+    public synchronized void close() {
         LOG.info("Config Loader service closed");
         this.closed = true;
         this.watcherThread.interrupt();
@@ -174,9 +209,11 @@ public final class ConfigLoaderImpl implements ConfigLoader, AutoCloseable {
             }
 
             if (key != null) {
-                for (final WatchEvent<?> event : key.pollEvents()) {
-                    handleEvent(event.context().toString());
-                }
+                final List<String> fileNames = key.pollEvents()
+                        .stream().map(event -> event.context().toString())
+                        .collect(Collectors.toList());
+                fileNames.forEach(this::handleEvent);
+
                 final boolean reset = key.reset();
                 if (!reset) {
                     LOG.warn("Could not reset the watch key.");
diff --git a/config-loader/config-loader-impl/src/test/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImplTest.java b/config-loader/config-loader-impl/src/test/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImplTest.java
index ae7dee5f1b..88e18bd020 100644
--- a/config-loader/config-loader-impl/src/test/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImplTest.java
+++ b/config-loader/config-loader-impl/src/test/java/org/opendaylight/bgpcep/config/loader/impl/ConfigLoaderImplTest.java
@@ -39,8 +39,9 @@ public class ConfigLoaderImplTest extends AbstractConfigLoader {
     }
 
     @Test
-    public void configLoaderImplTest() throws Exception {
-        assertNotNull(ClassLoader.getSystemClassLoader().getResource("etc/opendaylight/bgpcep/protocols-config.xml"));
+    public void configLoaderImplTest() {
+        assertNotNull(ClassLoader.getSystemClassLoader()
+                .getResource("etc/opendaylight/bgpcep/protocols-config.xml"));
         final AbstractRegistration ticket = this.configLoader.registerConfigFile(this.processor);
         verify(this.processor).loadConfiguration(any());
 
-- 
2.36.6