8257215: JFR: Events dropped when streaming over a chunk rotation

Reviewed-by: mgronlun

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -41,6 +41,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -851,4 +852,18 @@ public void removeBefore(Instant timestamp) {
         }
 
     }
+
+    public void removePath(SafePath path) {
+        synchronized (recorder) {
+            Iterator<RepositoryChunk> it = chunks.iterator();
+            while (it.hasNext()) {
+                RepositoryChunk c = it.next();
+                if (c.getFile().equals(path)) {
+                    it.remove();
+                    removed(c);
+                    return;
+                }
+            }
+        }
+    }
 }

src/jdk.jfr/share/classes/jdk/jfr/internal/Repository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
 import java.util.Set;
 
 import jdk.jfr.internal.SecuritySupport.SafePath;
+import jdk.jfr.internal.management.ChunkFilename;
 
 public final class Repository {
 
@@ -45,6 +46,7 @@ public final class Repository {
     private final Set<SafePath> cleanupDirectories = new HashSet<>();
     private SafePath baseLocation;
     private SafePath repository;
+    private ChunkFilename chunkFilename;
 
     private Repository() {
     }
@@ -61,6 +63,7 @@ public synchronized void setBasePath(SafePath baseLocation) throws IOException {
         // Probe to see if repository can be created, needed for fail fast
         // during JVM startup or JFR.configure
         this.repository = createRepository(baseLocation);
+        this.chunkFilename = null;
         try {
             // Remove so we don't "leak" repositories, if JFR is never started
             // and shutdown hook not added.
@@ -84,8 +87,13 @@ synchronized RepositoryChunk newChunk(ZonedDateTime timestamp) {
                 jvm.setRepositoryLocation(repository.toString());
                 SecuritySupport.setProperty(JFR_REPOSITORY_LOCATION_PROPERTY, repository.toString());
                 cleanupDirectories.add(repository);
+                chunkFilename = null;
             }
-            return new RepositoryChunk(repository, timestamp);
+            if (chunkFilename == null) {
+                chunkFilename = ChunkFilename.newPriviliged(repository.toPath());
+            }
+            String filename = chunkFilename.next(timestamp.toLocalDateTime());
+            return new RepositoryChunk(new SafePath(filename), timestamp.toInstant());
         } catch (Exception e) {
             String errorMsg = String.format("Could not create chunk in repository %s, %s: %s", repository, e.getClass(), e.getMessage());
             Logger.log(LogTag.JFR, LogLevel.ERROR, errorMsg);

src/jdk.jfr/share/classes/jdk/jfr/internal/RepositoryChunk.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -47,7 +47,6 @@ public int compare(RepositoryChunk c1, RepositoryChunk c2) {
         }
     };
 
-    private final SafePath repositoryPath;
     private final SafePath chunkFile;
     private final Instant startTime;
     private final RandomAccessFile unFinishedRAF;
@@ -56,28 +55,12 @@ public int compare(RepositoryChunk c1, RepositoryChunk c2) {
     private int refCount = 0;
     private long size;
 
-    RepositoryChunk(SafePath path, ZonedDateTime timestamp) throws Exception {
-        this.startTime = timestamp.toInstant();
-        this.repositoryPath = path;
-        this.chunkFile = findFileName(repositoryPath, timestamp.toLocalDateTime());
+    RepositoryChunk(SafePath path, Instant startTime) throws Exception {
+        this.startTime = startTime;
+        this.chunkFile = path;
         this.unFinishedRAF = SecuritySupport.createRandomAccessFile(chunkFile);
     }
 
-    private static SafePath findFileName(SafePath directory, LocalDateTime time) throws Exception {
-        String filename = Utils.formatDateTime(time);
-        Path p = directory.toPath().resolve(filename + FILE_EXTENSION);
-        for (int i = 1; i < MAX_CHUNK_NAMES; i++) {
-            SafePath s = new SafePath(p);
-            if (!SecuritySupport.exists(s)) {
-                return s;
-            }
-            String extendedName = String.format("%s_%02d%s", filename, i, FILE_EXTENSION);
-            p = directory.toPath().resolve(extendedName);
-        }
-        p = directory.toPath().resolve(filename + "_" + System.currentTimeMillis() + FILE_EXTENSION);
-        return new SafePath(p);
-    }
-
     void finish(Instant endTime) {
         try {
             finishWithException(endTime);

src/jdk.jfr/share/classes/jdk/jfr/internal/SecuritySupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -500,6 +500,11 @@ public long length(File f) throws IOException {
         public  long fileSize(Path p) throws IOException {
             return doPrivilegedIOWithReturn( () -> Files.size(p));
         }
+
+        @Override
+        public boolean exists(Path p) throws IOException {
+            return doPrivilegedIOWithReturn( () -> Files.exists(p));
+        }
     }
 
 

src/jdk.jfr/share/classes/jdk/jfr/internal/Utils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -184,7 +184,7 @@ public static String formatTimespan(Duration dValue, String separation) {
 
     // This method reduces the number of loaded classes
     // compared to DateTimeFormatter
-    static String formatDateTime(LocalDateTime time) {
+    public static String formatDateTime(LocalDateTime time) {
         StringBuilder sb = new StringBuilder(19);
         sb.append(time.getYear() / 100);
         appendPadded(sb, time.getYear() % 100, true);

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/FileAccess.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -46,6 +46,8 @@ public abstract class FileAccess {
 
     public abstract long fileSize(Path p) throws IOException;
 
+    public abstract boolean exists(Path s) throws IOException;
+
     private static class UnPrivileged extends FileAccess {
         @Override
         public RandomAccessFile openRAF(File f, String mode) throws IOException {
@@ -71,5 +73,10 @@ public long length(File f) throws IOException {
         public long fileSize(Path p) throws IOException {
             return Files.size(p);
         }
+
+        @Override
+        public boolean exists(Path p) {
+            return Files.exists(p);
+        }
     }
 }

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/OngoingStream.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -119,8 +119,7 @@ private byte[] readBytes() throws IOException {
                     throw new IOException("No progress");
                 }
                 startTimeNanos += header.getDurationNanos();
-                Instant timestamp = Utils.epochNanosToInstant(startTimeNanos);
-                ManagementSupport.removeBefore(recording, timestamp);
+                ManagementSupport.removePath(recording, path);
                 closeInput();
             } else {
                 header.refresh();

src/jdk.jfr/share/classes/jdk/jfr/internal/management/ChunkFilename.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.jfr.internal.management;
+
+import java.nio.file.Paths;
+import java.nio.file.Path;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
+
+import jdk.jfr.internal.SecuritySupport;
+import jdk.jfr.internal.SecuritySupport.SafePath;
+import jdk.jfr.internal.Utils;
+import jdk.jfr.internal.consumer.FileAccess;
+
+// Allows a remote streaming client to create chunk files
+// with same naming scheme as the JVM.
+public final class ChunkFilename {
+   private static final int MAX_CHUNK_NAMES = 100_000;
+   private static final String FILE_EXTENSION = ".jfr";
+
+   private final Path directory;
+   private final FileAccess fileAcess;
+
+   private Path lastPath;
+   private int counter;
+
+   public static ChunkFilename newUnpriviliged(Path directory) {
+       return new ChunkFilename(directory, FileAccess.UNPRIVILEGED);
+   }
+
+   public static ChunkFilename newPriviliged(Path directory) {
+       return new ChunkFilename(directory, SecuritySupport.PRIVILEGED);
+   }
+
+   private ChunkFilename(Path directory, FileAccess fileAccess) {
+       // Avoid malicious implementations of Path interface
+       this.directory = Paths.get(directory.toString());
+       this.fileAcess = fileAccess;
+   }
+
+   public String next(LocalDateTime time) throws IOException {
+       String filename = Utils.formatDateTime(time);
+       Path p = directory.resolve(filename + FILE_EXTENSION);
+
+       // If less than one file per second (typically case)
+       if (lastPath == null || !p.equals(lastPath)) {
+           if (!fileAcess.exists(p)) {
+               counter = 1; // reset counter
+               lastPath = p;
+               return p.toString();
+           }
+       }
+
+       // If more than one file per second
+       while (counter < MAX_CHUNK_NAMES) {
+           String extendedName = String.format("%s_%02d%s", filename, counter, FILE_EXTENSION);
+           p = directory.resolve(extendedName);
+           counter++;
+           if (!fileAcess.exists(p)) {
+               return p.toString();
+           }
+       }
+       throw new IOException("Unable to find unused filename after " + counter + " attempts");
+   }
+}

src/jdk.jfr/share/classes/jdk/jfr/internal/management/ManagementSupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -49,6 +49,7 @@
 import jdk.jfr.internal.MetadataRepository;
 import jdk.jfr.internal.PlatformRecording;
 import jdk.jfr.internal.PrivateAccess;
+import jdk.jfr.internal.SecuritySupport.SafePath;
 import jdk.jfr.internal.Utils;
 import jdk.jfr.internal.WriteableUserPath;
 import jdk.jfr.internal.consumer.EventDirectoryStream;
@@ -141,7 +142,12 @@ public static EventSettings newEventSettings(EventSettingsModifier esm) {
     public static void removeBefore(Recording recording, Instant timestamp) {
         PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
         pr.removeBefore(timestamp);
+    }
 
+    // Needed callback to detect when a chunk has been parsed.
+    public static void removePath(Recording recording, Path path) {
+        PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
+        pr.removePath(new SafePath(path));
     }
 
     // Needed callback to detect when a chunk has been parsed.