CI fails sporadically + clean up (#12)

---------

Co-authored-by: Tilmann Zäschke <tilmann.zaeschke@inf.ethz.ch>

CHANGELOG.md

@@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   [#9](https://github.com/tzaeschke/phtree-cpp/pull/9)
 
 ### Fixed
-- CI failures on JDK 8. [#10](https://github.com/tzaeschke/phtree-cpp/pull/10)
+- CI (only) failures on JDK 8. [#10](https://github.com/tzaeschke/phtree-cpp/pull/10)
+- Sporadic CI (only) failures. [#12](https://github.com/tzaeschke/phtree-cpp/pull/12)
 
 ## [0.1.0-ALPHA] - 2024-02-01
 

src/main/java/org/scion/ScionService.java

@@ -64,6 +64,7 @@ public class ScionService {
   private static final String DNS_TXT_KEY = "scion";
   private static final Object LOCK = new Object();
   private static ScionService DEFAULT = null;
+  private static final String ERR_INVALID_TXT = "Invalid TXT entry: ";
 
   private final ScionBootstrapper bootstrapper;
   // TODO create subclasses for these two? We can only have either one of them, not both.
@@ -89,7 +90,7 @@ protected ScionService(String addressOrHost, Mode mode) {
       daemonStub = DaemonServiceGrpc.newBlockingStub(channel);
       segmentStub = null;
       bootstrapper = null;
-      LOG.info("Path service started with daemon " + channel.toString() + " " + addressOrHost);
+      LOG.info("Path service started with daemon {} {}", channel, addressOrHost);
     } else {
       if (mode == Mode.BOOTSTRAP_VIA_DNS) {
         bootstrapper = ScionBootstrapper.createViaDns(addressOrHost);
@@ -107,10 +108,20 @@ protected ScionService(String addressOrHost, Mode mode) {
       channel = Grpc.newChannelBuilder(csHost, InsecureChannelCredentials.create()).build();
       daemonStub = null;
       segmentStub = SegmentLookupServiceGrpc.newBlockingStub(channel);
-      LOG.info("Path service started with control service " + channel.toString() + " " + csHost);
+      LOG.info("Path service started with control service {} {}", channel, csHost);
     }
     shutdownHook = addShutdownHook();
-    getLocalIsdAs(); // Init
+    try {
+      getLocalIsdAs(); // Init
+    } catch (RuntimeException e) {
+      // If this fails for whatever reason we want to make sure that the channel is closed.
+      try {
+        close();
+      } catch (IOException ex) {
+        // Ignore, we just want to get out.
+      }
+      throw e;
+    }
     synchronized (LOCK) {
       if (DEFAULT == null) {
         DEFAULT = this;
@@ -164,7 +175,7 @@ static ScionService defaultService() {
         // Ignore
       }
 
-      throw new ScionRuntimeException("Could not connect to daemon or bootstrap resource.");
+      throw new ScionRuntimeException("Could not connect to daemon, DNS or bootstrap resource.");
     }
   }
 
@@ -187,10 +198,12 @@ private Thread addShutdownHook() {
         new Thread(
             () -> {
               try {
-                DEFAULT.shutdownHook = null;
-                DEFAULT.close();
+                if (DEFAULT != null) {
+                  DEFAULT.shutdownHook = null;
+                  DEFAULT.close();
+                }
               } catch (IOException e) {
-                e.printStackTrace(System.err);
+                // Ignore, we just want to get out.
               }
             });
     Runtime.getRuntime().addShutdownHook(hook);
@@ -208,6 +221,7 @@ public void close() throws IOException {
         Runtime.getRuntime().removeShutdownHook(shutdownHook);
       }
     } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
       throw new IOException(e);
     }
   }
@@ -217,7 +231,6 @@ public DatagramChannel openChannel() throws IOException {
   }
 
   Daemon.ASResponse getASInfo() {
-    // LOG.info("*** GetASInfo ***");
     Daemon.ASRequest request = Daemon.ASRequest.newBuilder().setIsdAs(0).build();
     Daemon.ASResponse response;
     try {
@@ -232,7 +245,6 @@ Daemon.ASResponse getASInfo() {
   }
 
   Map<Long, Daemon.Interface> getInterfaces() {
-    // LOG.info("*** GetInterfaces ***");
     Daemon.InterfacesRequest request = Daemon.InterfacesRequest.newBuilder().build();
     Daemon.InterfacesResponse response;
     try {
@@ -324,7 +336,6 @@ public List<RequestPath> getPaths(long dstIsdAs, byte[] dstAddress, int dstPort)
   }
 
   Map<String, Daemon.ListService> getServices() throws ScionException {
-    // LOG.info("*** GetServices ***");
     Daemon.ServicesRequest request = Daemon.ServicesRequest.newBuilder().build();
     Daemon.ServicesResponse response;
     try {
@@ -440,29 +451,33 @@ private String findTxtRecordInProperties(String hostName, String key) throws Sci
         txtRecord = props.substring(posStart + 1);
       }
       if (!txtRecord.startsWith("\"" + key + "=") || !txtRecord.endsWith("\"")) {
-        throw new ScionException("Invalid TXT entry: " + txtRecord);
+        throw new ScionException(ERR_INVALID_TXT + txtRecord);
       }
       // No more checking here, we assume that properties are save
       return txtRecord.substring(key.length() + 2, txtRecord.length() - 1);
     }
     return null;
   }
 
-  private ScionAddress parseTxtRecord(String txtEntry, String hostName) {
+  private ScionAddress parseTxtRecord(String txtEntry, String hostName) throws ScionException {
     // dnsEntry example: "scion=64-2:0:9,129.132.230.98"
     int posComma = txtEntry.indexOf(',');
     if (posComma < 0) {
-      throw new ScionRuntimeException("Invalid TXT entry: " + txtEntry);
+      throw new ScionException(ERR_INVALID_TXT + txtEntry);
+    }
+    try {
+      long isdAs = ScionUtil.parseIA(txtEntry.substring(0, posComma));
+      return ScionAddress.create(isdAs, hostName, txtEntry.substring(posComma + 1));
+    } catch (IllegalArgumentException e) {
+      throw new ScionException(ERR_INVALID_TXT + txtEntry, e);
     }
-    long isdAs = ScionUtil.parseIA(txtEntry.substring(0, posComma));
-    return ScionAddress.create(isdAs, hostName, txtEntry.substring(posComma + 1));
   }
 
   private long parseTxtRecordToIA(String txtEntry) {
     // dnsEntry example: "scion=64-2:0:9,129.132.230.98"
     int posComma = txtEntry.indexOf(',');
     if (posComma < 0) {
-      throw new ScionRuntimeException("Invalid TXT entry: " + txtEntry);
+      throw new ScionRuntimeException(ERR_INVALID_TXT + txtEntry);
     }
     return ScionUtil.parseIA(txtEntry.substring(0, posComma));
   }

src/test/java/org/scion/ScionBootstrapperTest.java

@@ -17,12 +17,19 @@
 import static org.junit.jupiter.api.Assertions.*;
 
 import java.nio.file.Paths;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.scion.demo.DemoConstants;
 import org.scion.internal.ScionBootstrapper;
 
-public class ScionBootstrapperTest {
+class ScionBootstrapperTest {
+
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
 
   @Disabled // This requires access to the SCION production network
   @Test

src/test/java/org/scion/api/DatagramChannelApiTest.java

@@ -29,6 +29,7 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -54,6 +55,12 @@ public void afterEach() throws IOException {
     MockDNS.clear();
   }
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
   void getLocalAddress_withBind() throws IOException {
     InetSocketAddress addr = new InetSocketAddress("localhost", dummyPort);

src/test/java/org/scion/api/DatagramChannelMultiSendInetAddrTest.java

@@ -21,18 +21,24 @@
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramChannel;
 import org.scion.Path;
+import org.scion.ScionService;
 import org.scion.testutil.PingPongHelper;
 
 /** Test receive()/send(InetAddress) operations on DatagramChannel. */
 class DatagramChannelMultiSendInetAddrTest {
 
-  @Disabled
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = PingPongHelper::defaultServer;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 20, 50);

src/test/java/org/scion/api/DatagramChannelMultiSendPathTest.java

@@ -19,16 +19,24 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramChannel;
 import org.scion.Path;
+import org.scion.ScionService;
 import org.scion.testutil.PingPongHelper;
 
 /** Test receive()/send(Path) operations on DatagramChannel. */
 class DatagramChannelMultiSendPathTest {
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = PingPongHelper::defaultServer;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 20, 50);

src/test/java/org/scion/api/DatagramChannelMultiWriteConnectedInetSocketTest.java

@@ -21,16 +21,24 @@
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramChannel;
 import org.scion.Path;
+import org.scion.ScionService;
 import org.scion.testutil.PingPongHelper;
 
 /** Test read()/write() operations on DatagramChannel connected with an InetSocketAddress. */
 class DatagramChannelMultiWriteConnectedInetSocketTest {
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = PingPongHelper::defaultServer;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 10, 10);

src/test/java/org/scion/api/DatagramChannelMultiWriteConnectedPathTest.java

@@ -19,19 +19,27 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramChannel;
 import org.scion.Path;
 import org.scion.RequestPath;
+import org.scion.ScionService;
 import org.scion.testutil.PingPongHelper;
 
 /** Test read()/write() operations on DatagramChannel connected with a path. */
 class DatagramChannelMultiWriteConnectedPathTest {
 
   private static final String MSG = "Hello world!";
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = this::server;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 10, 10);

src/test/java/org/scion/api/DatagramChannelPacketValidationTest.java

@@ -25,9 +25,11 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramChannel;
+import org.scion.ScionService;
 import org.scion.ScionSocketOptions;
 import org.scion.internal.ScionHeaderParser;
 import org.scion.testutil.ExamplePacket;
@@ -51,6 +53,12 @@ public void beforeEach() {
     barrier = null;
   }
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
   void validate_length() {
     String PRE = "SCION packet validation failed: ";

src/test/java/org/scion/api/DatagramChannelPathSwitchTest.java

@@ -20,6 +20,7 @@
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.util.List;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.*;
 import org.scion.testutil.MockNetwork;
@@ -38,8 +39,14 @@ public RequestPath filter(List<RequestPath> paths) {
         }
       };
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = PingPongHelper::defaultServer;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 2, 10);

src/test/java/org/scion/api/DatagramChannelStreamTest.java

@@ -20,6 +20,7 @@
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 import org.scion.*;
 import org.scion.testutil.MockNetwork;
@@ -30,8 +31,14 @@ class DatagramChannelStreamTest {
 
   private static final int N_BULK = 10;
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
-  public void test() {
+  void test() {
     PingPongHelper.Server serverFn = this::server;
     PingPongHelper.Client clientFn = this::client;
     PingPongHelper pph = new PingPongHelper(1, 2, 2);

src/test/java/org/scion/api/DatagramSocketPingPongTest.java

@@ -19,9 +19,11 @@
 import java.io.IOException;
 import java.net.*;
 import java.util.ArrayList;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.scion.DatagramSocket;
+import org.scion.ScionService;
 import org.scion.testutil.MockNetwork;
 
 @Disabled // TODO this does not work anymore.
@@ -35,6 +37,12 @@ class DatagramSocketPingPongTest {
   private int nServer = 0;
   private final ArrayList<Throwable> exceptions = new ArrayList<>();
 
+  @AfterAll
+  public static void afterAll() {
+    // Defensive clean up
+    ScionService.closeDefault();
+  }
+
   @Test
   void testPingPong() throws InterruptedException {
     MockNetwork.startTiny();