+{"files":[{"patch":"@@ -51,0 +51,4 @@\n+#if defined(LINUX)\n+# include <sys\/file.h>\n+#endif\n+\n@@ -79,12 +83,0 @@\n-\/\/ delete the PerfData memory region\n-\/\/\n-static void delete_standard_memory(char* addr, size_t size) {\n-\n- \/\/ there are no persistent external resources to cleanup for standard\n- \/\/ memory. since DestroyJavaVM does not support unloading of the JVM,\n- \/\/ cleanup of the memory resource is not performed. The memory will be\n- \/\/ reclaimed by the OS upon termination of the process.\n- \/\/\n- return;\n-}\n-\n@@ -710,2 +702,1 @@\n-\n-\/\/ cleanup stale shared memory resources\n+\/\/ cleanup stale shared memory files\n@@ -715,4 +706,5 @@\n-\/\/ for files matching the pattern ^$[0-9]*$. For each file found, the\n-\/\/ process id is extracted from the file name and a test is run to\n-\/\/ determine if the process is alive. If the process is not alive,\n-\/\/ any stale file resources are removed.\n+\/\/ for files matching the pattern ^$[0-9]*$.\n+\/\/\n+\/\/ This directory should be used only by JVM processes owned by the\n+\/\/ current user to store PerfMemory files. Any other files found\n+\/\/ in this directory may be removed.\n@@ -720,1 +712,1 @@\n-static void cleanup_sharedmem_resources(const char* dirname) {\n+static void cleanup_sharedmem_files(const char* dirname) {\n@@ -730,7 +722,6 @@\n- \/\/ for each entry in the directory that matches the expected file\n- \/\/ name pattern, determine if the file resources are stale and if\n- \/\/ so, remove the file resources. Note, instrumented HotSpot processes\n- \/\/ for this user may start and\/or terminate during this search and\n- \/\/ remove or create new files in this directory. The behavior of this\n- \/\/ loop under these conditions is dependent upon the implementation of\n- \/\/ opendir\/readdir.\n+ \/\/ For each entry in the directory that matches the expected file\n+ \/\/ name pattern, remove the file if it's determine to be stale\n+ \/\/ Note, instrumented HotSpot processes for this user may start and\/or\n+ \/\/ terminate during this search and remove or create new files in this\n+ \/\/ directory. The behavior of this loop under these conditions is dependent\n+ \/\/ upon the implementation of opendir\/readdir.\n@@ -741,2 +732,2 @@\n-\n- pid_t pid = filename_to_pid(entry->d_name);\n+ const char* filename = entry->d_name;\n+ pid_t pid = filename_to_pid(filename);\n@@ -745,2 +736,1 @@\n-\n- if (strcmp(entry->d_name, \".\") != 0 && strcmp(entry->d_name, \"..\") != 0) {\n+ if (strcmp(filename, \".\") != 0 && strcmp(filename, \"..\") != 0) {\n@@ -748,1 +738,1 @@\n- unlink(entry->d_name);\n+ unlink(filename);\n@@ -755,12 +745,3 @@\n- \/\/ we now have a file name that converts to a valid integer\n- \/\/ that could represent a process id . if this process id\n- \/\/ matches the current process id or the process is not running,\n- \/\/ then remove the stale file resources.\n- \/\/\n- \/\/ process liveness is detected by sending signal number 0 to\n- \/\/ the process id (see kill(2)). if kill determines that the\n- \/\/ process does not exist, then the file resources are removed.\n- \/\/ if kill determines that that we don't have permission to\n- \/\/ signal the process, then the file resources are assumed to\n- \/\/ be stale and are removed because the resources for such a\n- \/\/ process should be in a different user specific directory.\n+#if defined(LINUX)\n+ \/\/ Special case on Linux, if multiple containers share the\n+ \/\/ same \/tmp directory:\n@@ -768,3 +749,13 @@\n- if ((pid == os::current_process_id()) ||\n- (kill(pid, 0) == OS_ERR && (errno == ESRCH || errno == EPERM))) {\n- unlink(entry->d_name);\n+ \/\/ - All the JVMs must have the JDK-8286030 fix, or the behavior\n+ \/\/ is undefined.\n+ \/\/ - We cannot rely on the values of the pid, because it could\n+ \/\/ be a process in a different namespace. We must use the flock\n+ \/\/ protocol to determine if a live process is using this file.\n+ \/\/ See create_sharedmem_file().\n+ int fd;\n+ RESTARTABLE(os::open(filename, O_RDONLY, 0), fd);\n+ if (fd == OS_ERR) {\n+ \/\/ Something wrong happened. Ignore the error and don't try to remove the\n+ \/\/ file.\n+ errno = 0;\n+ continue;\n@@ -772,0 +763,47 @@\n+\n+ int n;\n+ RESTARTABLE(::flock(fd, LOCK_EX|LOCK_NB), n);\n+ if (n != 0) {\n+ \/\/ Either another process holds the exclusive lock on this file, or\n+ \/\/ something wrong happened. Ignore the error and don't try to remove the\n+ \/\/ file.\n+ log_debug(perf, memops)(\"flock for stale file check failed for %s\/%s\", dirname, filename);\n+ ::close(fd);\n+ errno = 0;\n+ continue;\n+ }\n+ \/\/ We are able to lock the file, but this file might have been created\n+ \/\/ by an older JVM that doesn't use the flock prototol, so we must do\n+ \/\/ the folowing checks (which are also done by older JVMs).\n+#endif\n+\n+ \/\/ The following code assumes that pid must be in the same\n+ \/\/ namespace as the current process.\n+ bool stale = false;\n+\n+ if (pid == os::current_process_id()) {\n+ \/\/ The file was created by a terminated process that happened\n+ \/\/ to have the same pid as the current process.\n+ stale = true;\n+ } else if (kill(pid, 0) == OS_ERR) {\n+ if (errno == ESRCH) {\n+ \/\/ The target process does not exist.\n+ stale = true;\n+ } else if (errno == EPERM) {\n+ \/\/ The file was created by a terminated process that happened\n+ \/\/ to have the same pid as a process not owned by the current user.\n+ stale = true;\n+ }\n+ }\n+\n+ if (stale) {\n+ log_info(perf, memops)(\"Remove stale file %s\/%s\", dirname, filename);\n+ unlink(filename);\n+ }\n+\n+ #if defined(LINUX)\n+ \/\/ Hold the lock until here to prevent other JVMs from using this file\n+ \/\/ while we are in the middle of deleting it.\n+ ::close(fd);\n+#endif\n+\n@@ -817,1 +855,1 @@\n-\/\/ create the shared memory file resources\n+\/\/ create the shared memory file\n@@ -823,1 +861,1 @@\n-static int create_sharedmem_resources(const char* dirname, const char* filename, size_t size) {\n+static int create_sharedmem_file(const char* dirname, const char* filename, size_t size) {\n@@ -868,0 +906,26 @@\n+#if defined(LINUX)\n+ \/\/ On Linux, different containerized processes that share the same \/tmp\n+ \/\/ directory (e.g., with \"docker --volume ...\") may have the same pid and\n+ \/\/ try to use the same file. To avoid conflicts among such\n+ \/\/ processes, we allow only one of them (the winner of the flock() call)\n+ \/\/ to write to the file. All the other processes will give up and will\n+ \/\/ have perfdata disabled.\n+ \/\/\n+ \/\/ Note that the flock will be automatically given up when the winner\n+ \/\/ process exits.\n+ \/\/\n+ \/\/ The locking protocol works only with other JVMs that have the JDK-8286030\n+ \/\/ fix. If you are sharing the \/tmp difrectory among different containers,\n+ \/\/ do not use older JVMs that don't have this fix, or the behavior is undefined.\n+ int n;\n+ RESTARTABLE(::flock(fd, LOCK_EX|LOCK_NB), n);\n+ if (n != 0) {\n+ log_warning(perf, memops)(\"Cannot use file %s\/%s because %s\", dirname, filename,\n+ (errno == EWOULDBLOCK) ?\n+ \"it is locked by another process\" :\n+ \"flock() failed\");\n+ ::close(fd);\n+ return -1;\n+ }\n+#endif\n+\n@@ -984,1 +1048,1 @@\n- cleanup_sharedmem_resources(dirname);\n+ cleanup_sharedmem_files(dirname);\n@@ -989,1 +1053,2 @@\n- fd = create_sharedmem_resources(dirname, short_filename, size);\n+ log_info(perf, memops)(\"Trying to open %s\/%s\", dirname, short_filename);\n+ fd = create_sharedmem_file(dirname, short_filename, size);\n@@ -1022,0 +1087,2 @@\n+ log_info(perf, memops)(\"Successfully opened\");\n+\n@@ -1056,2 +1123,2 @@\n- \/\/ cleanup the persistent shared memory resources. since DestroyJavaVM does\n- \/\/ not support unloading of the JVM, unmapping of the memory resource is\n+ \/\/ Remove the shared memory file. Since DestroyJavaVM does\n+ \/\/ not support unloading of the JVM, unmapping of the memory region is\n@@ -1059,1 +1126,1 @@\n- \/\/ the process. The backing store file is deleted from the file system.\n+ \/\/ the process.\n@@ -1235,4 +1302,1 @@\n- if (PerfDisableSharedMem) {\n- delete_standard_memory(start(), capacity());\n- }\n- else {\n+ if (!PerfDisableSharedMem) {\n","filename":"src\/hotspot\/os\/posix\/perfMemory_posix.cpp","additions":121,"deletions":57,"binary":false,"changes":178,"status":"modified"},{"patch":"@@ -0,0 +1,132 @@\n+\/*\n+ * Copyright (c) 2022, Oracle and\/or its affiliates. All rights reserved.\n+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.\n+ *\n+ * This code is free software; you can redistribute it and\/or modify it\n+ * under the terms of the GNU General Public License version 2 only, as\n+ * published by the Free Software Foundation.\n+ *\n+ * This code is distributed in the hope that it will be useful, but WITHOUT\n+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or\n+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License\n+ * version 2 for more details (a copy is included in the LICENSE file that\n+ * accompanied this code).\n+ *\n+ * You should have received a copy of the GNU General Public License version\n+ * 2 along with this work; if not, write to the Free Software Foundation,\n+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.\n+ *\n+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA\n+ * or visit www.oracle.com if you need additional information or have any\n+ * questions.\n+ *\/\n+\n+\n+\/*\n+ * @test\n+ * @bug 8286030\n+ * @key cgroups\n+ * @summary Test for hsperfdata file name conflict when two containers share the same \/tmp directory\n+ * @requires docker.support\n+ * @library \/test\/lib\n+ * @build WaitForFlagFile\n+ * @run driver ShareTmpDir\n+ *\/\n+\n+import java.io.File;\n+import java.io.FileOutputStream;\n+import java.util.regex.Pattern;\n+import java.util.regex.Matcher;\n+import jdk.test.lib.Asserts;\n+import jdk.test.lib.Utils;\n+import jdk.test.lib.containers.docker.Common;\n+import jdk.test.lib.containers.docker.DockerRunOptions;\n+import jdk.test.lib.containers.docker.DockerTestUtils;\n+import jdk.test.lib.process.OutputAnalyzer;\n+import jtreg.SkippedException;\n+\n+public class ShareTmpDir {\n+ private static final String imageName = Common.imageName(\"sharetmpdir\");\n+\n+ public static void main(String[] args) throws Exception {\n+ if (!DockerTestUtils.canTestDocker()) {\n+ return;\n+ }\n+\n+ DockerTestUtils.buildJdkContainerImage(imageName);\n+\n+ try {\n+ test();\n+ } finally {\n+ if (!DockerTestUtils.RETAIN_IMAGE_AFTER_TEST) {\n+ DockerTestUtils.removeDockerImage(imageName);\n+ }\n+ }\n+ }\n+\n+ static OutputAnalyzer out1, out2;\n+\n+ private static void test() throws Exception {\n+ File sharedtmpdir = new File(\"sharedtmpdir\");\n+ File flag = new File(sharedtmpdir, \"flag\");\n+ File started = new File(sharedtmpdir, \"started\");\n+ sharedtmpdir.mkdir();\n+ flag.delete();\n+ started.delete();\n+ DockerRunOptions opts = new DockerRunOptions(imageName, \"\/jdk\/bin\/java\", \"WaitForFlagFile\");\n+ opts.addDockerOpts(\"--volume\", Utils.TEST_CLASSES + \":\/test-classes\/\");\n+ opts.addDockerOpts(\"--volume\", sharedtmpdir.getAbsolutePath() + \":\/tmp\/\");\n+ opts.addJavaOpts(\"-Xlog:os+container=trace\", \"-Xlog:perf+memops=debug\", \"-cp\", \"\/test-classes\/\");\n+\n+ Thread t1 = new Thread() {\n+ public void run() {\n+ try { out1 = Common.run(opts); } catch (Exception e) { e.printStackTrace(); }\n+ }\n+ };\n+ t1.start();\n+\n+ Thread t2 = new Thread() {\n+ public void run() {\n+ try { out2 = Common.run(opts); } catch (Exception e) { e.printStackTrace(); }\n+ }\n+ };\n+ t2.start();\n+\n+ while (!started.exists()) {\n+ System.out.println(\"Wait for at least one JVM to start\");\n+ Thread.sleep(1000);\n+ }\n+\n+ \/\/ Set the flag for the two JVMs to exit\n+ FileOutputStream fout = new FileOutputStream(flag);\n+ fout.close();\n+\n+ t1.join();\n+ t2.join();\n+\n+ Pattern pattern = Pattern.compile(\"perf,memops.*Trying to open (\/tmp\/hsperfdata_[a-z0-9]*\/[0-9]*)\");\n+ Matcher matcher;\n+\n+ matcher = pattern.matcher(out1.getStdout());\n+ Asserts.assertTrue(matcher.find());\n+ String file1 = matcher.group(1);\n+\n+ matcher = pattern.matcher(out2.getStdout());\n+ Asserts.assertTrue(matcher.find());\n+ String file2 = matcher.group(1);\n+\n+ Asserts.assertTrue(file1 != null);\n+ Asserts.assertTrue(file2 != null);\n+\n+ if (file1.equals(file2)) {\n+ \/\/ This should be the common case -- the first started process in a container should\n+ \/\/ have pid==1.\n+ \/\/ One of the two contains must fail to create the hsperf file.\n+ String s = \"Cannot use file \" + file1 + \" because it is locked by another process\";\n+ Asserts.assertTrue(out1.getStdout().contains(s) ||\n+ out2.getStdout().contains(s));\n+ } else {\n+ throw new SkippedException(\"Java in the two containers don't have the same pid: \" + file1 + \" vs \" + file2);\n+ }\n+ }\n+}\n","filename":"test\/hotspot\/jtreg\/containers\/docker\/ShareTmpDir.java","additions":132,"deletions":0,"binary":false,"changes":132,"status":"added"},{"patch":"@@ -0,0 +1,43 @@\n+\/*\n+ * Copyright (c) 2022, Oracle and\/or its affiliates. All rights reserved.\n+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.\n+ *\n+ * This code is free software; you can redistribute it and\/or modify it\n+ * under the terms of the GNU General Public License version 2 only, as\n+ * published by the Free Software Foundation.\n+ *\n+ * This code is distributed in the hope that it will be useful, but WITHOUT\n+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or\n+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License\n+ * version 2 for more details (a copy is included in the LICENSE file that\n+ * accompanied this code).\n+ *\n+ * You should have received a copy of the GNU General Public License version\n+ * 2 along with this work; if not, write to the Free Software Foundation,\n+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.\n+ *\n+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA\n+ * or visit www.oracle.com if you need additional information or have any\n+ * questions.\n+ *\/\n+\n+import java.io.File;\n+import java.io.FileOutputStream;\n+\n+public class WaitForFlagFile {\n+ public static void main(String[] args) throws Exception {\n+ System.out.println(\"WaitForFlagFile: Entering\");\n+\n+ File started = new File(\"\/tmp\/started\");\n+ FileOutputStream fout = new FileOutputStream(started);\n+ fout.close();\n+\n+ File flag = new File(\"\/tmp\/flag\");\n+ while (!flag.exists()) {\n+ System.out.println(\"WaitForFlagFile: Waiting\");\n+ Thread.sleep(500);\n+ }\n+ System.out.println(\"WaitForFlagFile: Exiting\");\n+\n+ }\n+}\n","filename":"test\/hotspot\/jtreg\/containers\/docker\/WaitForFlagFile.java","additions":43,"deletions":0,"binary":false,"changes":43,"status":"added"}]}
0 commit comments