From 72c7d0f64d11cc6e2d16cc16e0553b66186ae9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BD=9B=E7=B3=BB=E5=B0=91=E5=B9=B4=E4=B8=AD=E4=BA=8C?= Date: Thu, 12 Dec 2024 20:23:29 +0800 Subject: [PATCH] IBAYNJ: Fix a concurrent issue of program data ref cnt --- ...urrent-issue-of-program-data-ref-cnt.patch | 363 ++++++++++++++++++ openjdk-17.spec | 7 +- 2 files changed, 369 insertions(+), 1 deletion(-) create mode 100644 Fix-a-concurrent-issue-of-program-data-ref-cnt.patch diff --git a/Fix-a-concurrent-issue-of-program-data-ref-cnt.patch b/Fix-a-concurrent-issue-of-program-data-ref-cnt.patch new file mode 100644 index 0000000..08d497e --- /dev/null +++ b/Fix-a-concurrent-issue-of-program-data-ref-cnt.patch @@ -0,0 +1,363 @@ +From b084e0e23535cb7e46802c3bf471ee8cafbe8a74 Mon Sep 17 00:00:00 2001 +Subject: Fix a concurrent issue of program data ref cnt + +--- + .../share/jbooster/jClientArguments.hpp | 4 + + .../jbooster/server/serverDataManager.hpp | 4 + + .../utilities/concurrentHashMap.inline.hpp | 20 +--- + test/hotspot/gtest/jbooster/test_net.cpp | 1 + + test/hotspot/gtest/jbooster/test_server.cpp | 107 ++++++++++++++++++ + test/hotspot/gtest/jbooster/test_util.cpp | 3 +- + .../jbooster/JBoosterConcurrentTest.java | 106 +++++++++++++++++ + test/jdk/tools/jbooster/TEST.properties | 1 + + 8 files changed, 231 insertions(+), 15 deletions(-) + create mode 100644 test/hotspot/gtest/jbooster/test_server.cpp + create mode 100644 test/jdk/tools/jbooster/JBoosterConcurrentTest.java + create mode 100644 test/jdk/tools/jbooster/TEST.properties + +diff --git a/src/hotspot/share/jbooster/jClientArguments.hpp b/src/hotspot/share/jbooster/jClientArguments.hpp +index 491ac3159..dd56ddc36 100644 +--- a/src/hotspot/share/jbooster/jClientArguments.hpp ++++ b/src/hotspot/share/jbooster/jClientArguments.hpp +@@ -60,6 +60,9 @@ DECLARE_SERIALIZER_INTRUSIVE(JClientBoostLevel); + * Arguments that identify a program. + */ + class JClientArguments final: public CHeapObj { ++ // @see test/hotspot/gtest/jbooster/test_server.cpp ++ friend class Gtest_JBoosterServer; ++ + enum InitState { + NOT_INITIALIZED_FOR_SEREVR, + INITIALIZED_FOR_SEREVR, +@@ -103,6 +106,7 @@ private: + + public: + JClientArguments(bool is_client); ++ JClientArguments(DebugUtils* ignored) {} // for GTEST only + ~JClientArguments(); + + int serialize(MessageBuffer& buf) const; +diff --git a/src/hotspot/share/jbooster/server/serverDataManager.hpp b/src/hotspot/share/jbooster/server/serverDataManager.hpp +index c6ef4c99a..a53af341c 100644 +--- a/src/hotspot/share/jbooster/server/serverDataManager.hpp ++++ b/src/hotspot/share/jbooster/server/serverDataManager.hpp +@@ -173,6 +173,9 @@ public: + * The data of the program (usually a .class or .jar file) executed by the client. + */ + class JClientProgramData final: public CHeapObj { ++ // @see test/hotspot/gtest/jbooster/test_server.cpp ++ friend class Gtest_JBoosterServer; ++ + public: + using ClassLoaderTable = ConcurrentHashMap; + +@@ -195,6 +198,7 @@ private: + + public: + JClientProgramData(uint32_t program_id, JClientArguments* program_args); ++ JClientProgramData(DebugUtils* ignored) {} // for GTEST only + ~JClientProgramData(); + + uint32_t program_id() const { return _program_id; } +diff --git a/src/hotspot/share/jbooster/utilities/concurrentHashMap.inline.hpp b/src/hotspot/share/jbooster/utilities/concurrentHashMap.inline.hpp +index e69da290f..8abb853b7 100644 +--- a/src/hotspot/share/jbooster/utilities/concurrentHashMap.inline.hpp ++++ b/src/hotspot/share/jbooster/utilities/concurrentHashMap.inline.hpp +@@ -168,8 +168,8 @@ inline V* ConcurrentHashMap::get(const K& key, Thread* thread) + V* res = nullptr; + + if (_table.get(thread, lookup, get, &grow_hint)) { +- KVNode& kv_node = *get.res(); +- res = &kv_node.value(); ++ assert(get.res() != nullptr, "sanity"); ++ res = &(get.res()->value()); + } + + return res; +@@ -184,18 +184,10 @@ inline V* ConcurrentHashMap::put_if_absent(const K& key, V& val + + V* res = nullptr; + +- do { +- if (_table.insert(thread, lookup, KVNode(key, value), &grow_hint, &clean_hint)) { +- res = &value; +- break; +- } +- +- if (_table.get(thread, lookup, get, &grow_hint)) { +- KVNode& kv_node = *get.res(); +- res = &kv_node.value(); +- break; +- } +- } while (true); ++ KVNode kv_node(key, value); ++ bool success = _table.insert_get(thread, lookup, kv_node, get, &grow_hint, &clean_hint); ++ assert(get.res() != nullptr, "sanity"); ++ res = &(get.res()->value()); + + if (grow_hint) { + grow_if_needed(thread); +diff --git a/test/hotspot/gtest/jbooster/test_net.cpp b/test/hotspot/gtest/jbooster/test_net.cpp +index dd45dd65a..336418cf4 100644 +--- a/test/hotspot/gtest/jbooster/test_net.cpp ++++ b/test/hotspot/gtest/jbooster/test_net.cpp +@@ -37,6 +37,7 @@ + #include "runtime/os.inline.hpp" + #include "utilities/globalDefinitions.hpp" + #include "utilities/growableArray.hpp" ++ + #include "unittest.hpp" + + static int try_catch_func(int i) { +diff --git a/test/hotspot/gtest/jbooster/test_server.cpp b/test/hotspot/gtest/jbooster/test_server.cpp +new file mode 100644 +index 000000000..322b947fd +--- /dev/null ++++ b/test/hotspot/gtest/jbooster/test_server.cpp +@@ -0,0 +1,107 @@ ++/* ++ * Copyright (c) 2020, 2024, Huawei Technologies Co., Ltd. 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. ++ * ++ * 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. ++ */ ++ ++#include "precompiled.hpp" ++#include "utilities/macros.hpp" ++ ++#if INCLUDE_JBOOSTER ++ ++#include "common.hpp" ++ ++#include "jbooster/jClientArguments.hpp" ++#include "jbooster/server/serverDataManager.hpp" ++#include "jbooster/utilities/concurrentHashMap.inline.hpp" ++#include "jbooster/utilities/debugUtils.inline.hpp" ++#include "utilities/stringUtils.hpp" ++ ++#include "unittest.hpp" ++ ++struct Gtest_JBoosterServer { ++ static JClientArguments* create_mock_JClientArguments(uint32_t id) { ++ JClientArguments* mock = new JClientArguments((DebugUtils*) nullptr); ++ ++ // [FOR EACH ARG] ++ mock->_cpu_arch = JClientArguments::CpuArch::CPU_UNKOWN; ++ mock->_jvm_version = id; ++ mock->_internal_jvm_info = StringUtils::copy_to_heap("internaljvminfo", mtJBooster); ++ mock->_program_name = StringUtils::copy_to_heap("programname", mtJBooster); ++ mock->_program_entry = StringUtils::copy_to_heap("programentry", mtJBooster); ++ mock->_is_jar = true; ++ mock->_classpath_name_hash = 1u; ++ mock->_classpath_timestamp_hash = 2u; ++ mock->_agent_name_hash = 3u; ++ mock->_cds_file_size = 456; ++ mock->_java_commands = StringUtils::copy_to_heap("javacommands", mtJBooster); ++ mock->_related_flags = new JClientVMFlags(true); ++ ++ mock->_hash = mock->calc_hash(); ++ mock->_state = JClientArguments::INITIALIZED_FOR_SEREVR; ++ ++ return mock; ++ } ++ ++ static JClientProgramData* create_mock_JClientProgramData(uint32_t program_id, JClientArguments* program_args) { ++ JClientProgramData* mock = new JClientProgramData((DebugUtils*) nullptr); ++ ++ mock->_program_id = program_id; ++ mock->_program_args = program_args->move(); ++ mock->_ref_cnt.inc(); ++ mock->_program_str_id = StringUtils::copy_to_heap("programstrid", mtJBooster); ++ mock->clr_cache_state().init(StringUtils::copy_to_heap("gtestclr", mtJBooster)); ++ mock->dy_cds_cache_state().init(StringUtils::copy_to_heap("gtestdycds", mtJBooster)); ++ mock->agg_cds_cache_state().init(StringUtils::copy_to_heap("gtestaggcds", mtJBooster)); ++ mock->aot_static_cache_state().init(StringUtils::copy_to_heap("gtestaotstatic", mtJBooster)); ++ mock->aot_pgo_cache_state().init(StringUtils::copy_to_heap("gtestaotpgo", mtJBooster)); ++ ++ return mock; ++ } ++}; ++ ++TEST_VM(JBoosterServer, program_data_ref_cnt) { ++ JavaThread* THREAD = JavaThread::current(); ++ ServerDataManager::JClientProgramDataMap programs; ++ JClientArguments* program_args1 = Gtest_JBoosterServer::create_mock_JClientArguments(11); ++ JClientProgramData* new_pd1 = Gtest_JBoosterServer::create_mock_JClientProgramData(1u, program_args1); ++ delete program_args1; ++ JClientProgramData** res1 = programs.put_if_absent(new_pd1->program_args(), new_pd1, THREAD); ++ EXPECT_NE(res1, (JClientProgramData**) nullptr); ++ EXPECT_EQ(*res1, new_pd1); ++ EXPECT_EQ(new_pd1->ref_cnt().get(), 1); ++ ++ JClientArguments* program_args2 = Gtest_JBoosterServer::create_mock_JClientArguments(11); ++ JClientProgramData* new_pd2 = Gtest_JBoosterServer::create_mock_JClientProgramData(2u, program_args2); ++ delete program_args2; ++ JClientProgramData** res2 = programs.put_if_absent(new_pd2->program_args(), new_pd2, THREAD); ++ EXPECT_NE(res2, (JClientProgramData**) nullptr); ++ EXPECT_EQ(*res2, new_pd1); ++ EXPECT_EQ(new_pd1->ref_cnt().get(), 2); ++ EXPECT_EQ(new_pd2->ref_cnt().get(), 1); ++ ++ new_pd2->ref_cnt().dec_and_update_time(); ++ delete new_pd2; ++ ++ new_pd1->ref_cnt().dec_and_update_time(); ++ new_pd1->ref_cnt().dec_and_update_time(); ++} ++ ++#endif // INCLUDE_JBOOSTER +diff --git a/test/hotspot/gtest/jbooster/test_util.cpp b/test/hotspot/gtest/jbooster/test_util.cpp +index 461e3faa7..178998189 100644 +--- a/test/hotspot/gtest/jbooster/test_util.cpp ++++ b/test/hotspot/gtest/jbooster/test_util.cpp +@@ -35,10 +35,11 @@ + #include "jbooster/utilities/scalarHashMap.inline.hpp" + #include "runtime/os.inline.hpp" + #include "runtime/thread.hpp" +-#include "unittest.hpp" + #include "utilities/exceptions.hpp" + #include "utilities/stringUtils.hpp" + ++#include "unittest.hpp" ++ + class ATestClass {}; + + template +diff --git a/test/jdk/tools/jbooster/JBoosterConcurrentTest.java b/test/jdk/tools/jbooster/JBoosterConcurrentTest.java +new file mode 100644 +index 000000000..56423ba82 +--- /dev/null ++++ b/test/jdk/tools/jbooster/JBoosterConcurrentTest.java +@@ -0,0 +1,106 @@ ++/* ++ * Copyright (c) 2020, 2023, Huawei Technologies Co., Ltd. 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. ++ * ++ * 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. ++ */ ++ ++import java.net.ServerSocket; ++import java.util.concurrent.TimeUnit; ++import java.util.stream.Collectors; ++import java.util.stream.Stream; ++import java.util.ArrayList; ++import java.util.List; ++ ++import jdk.test.lib.process.OutputAnalyzer; ++ ++import static jdk.test.lib.Asserts.*; ++ ++/* ++ * jbooster testing. ++ * @test ++ * @summary Test jbooster server ++ * @library ../lib ++ * /test/lib ++ * @modules jdk.jbooster ++ * @build SimpleClient ++ * @run main/othervm/timeout=5000 JBoosterConcurrentTest ++ */ ++public class JBoosterConcurrentTest extends JBoosterTestBase { ++ public static final List SERVER_FAST_CLEANUP_ARGS = Stream.concat(SERVER_STANDARD_ARGS.stream(), Stream.of( ++ "--unused-cleanup-timeout=1" ++ )).collect(Collectors.toList()); ++ ++ private static void testProgramDataCleanup(TestContext ctx) throws Exception { ++ Process server = jbooster(ctx, List.of(), SERVER_FAST_CLEANUP_ARGS); ++ OutputAnalyzer out = new OutputAnalyzer(server); ++ server.waitFor(WAIT_START_TIME, TimeUnit.SECONDS); ++ ++ List apps = new ArrayList<>(); ++ final int rounds = 100; ++ final int concurrent = 5; ++ for (int r = 0; r < rounds; ++r) { ++ if (r % 10 == 0) { ++ System.out.println("Try " + (r + 1) + "round:"); ++ } ++ ++ ArrayList clientArgs = new ArrayList<>(CLIENT_STANDARD_ARGS); ++ clientArgs.add(clientArgs.size() - 1, "-XX:JBoosterProgramName=simple-" + r); ++ ++ for (int j = 0; j < concurrent; ++j) { ++ Process p = java(ctx, clientArgs); ++ apps.add(p); ++ } ++ ++ if (r % 10 == 0) { ++ for (Process p : apps) { ++ p.waitFor(WAIT_EXIT_TIME, TimeUnit.SECONDS); ++ assertEquals(p.exitValue(), 0, "good"); ++ } ++ apps.clear(); ++ } ++ } ++ ++ System.out.println("Get data 1:"); ++ writeToStdin(server, "d\n"); ++ ++ for (Process p : apps) { ++ p.waitFor(WAIT_EXIT_TIME, TimeUnit.SECONDS); ++ assertEquals(p.exitValue(), 0, "good"); ++ } ++ apps.clear(); ++ ++ System.out.println("Get data 2:"); ++ writeToStdin(server, "d\n"); ++ ++ Thread.sleep(5 * 60 * 1000); ++ ++ System.out.println("Get data 3:"); ++ writeToStdin(server, "d\n"); ++ writeToStdin(server, "d\n"); ++ writeToStdin(server, "d\n"); ++ exitNormallyByCommandQ(server); ++ out.shouldContain("JClientProgramData:\n -"); ++ out.shouldContain("JClientSessionData:\n\nJClientProgramData:\n\n".repeat(4)); ++ } ++ ++ public static void main(String[] args) throws Exception { ++ testCases(JBoosterConcurrentTest.class); ++ } ++} +diff --git a/test/jdk/tools/jbooster/TEST.properties b/test/jdk/tools/jbooster/TEST.properties +new file mode 100644 +index 000000000..e11b8706b +--- /dev/null ++++ b/test/jdk/tools/jbooster/TEST.properties +@@ -0,0 +1 @@ ++maxOutputSize = 2500000 +-- +2.22.0 + diff --git a/openjdk-17.spec b/openjdk-17.spec index f064940..6de3f0e 100644 --- a/openjdk-17.spec +++ b/openjdk-17.spec @@ -914,7 +914,7 @@ Provides: java-src%{?1} = %{epoch}:%{version}-%{release} Name: java-%{javaver}-%{origin} Version: %{newjavaver}.%{buildver} -Release: 5 +Release: 6 # java-1.5.0-ibm from jpackage.org set Epoch to 1 for unknown reasons # and this change was brought into RHEL-4. java-1.5.0-ibm packages @@ -1060,6 +1060,7 @@ Patch74: Optimize-LazyAOT-klasses-sending-strategy.patch Patch75: Add-KAE-zip-GzipKAEBenchmark-Benchmark.patch Patch76: Add-Fix-clear-mark-for-NativeGotJump.patch Patch77: 8323066-TestSkipRebuildRemsetPhase.java-fails-with-S.patch +Patch78: Fix-a-concurrent-issue-of-program-data-ref-cnt.patch ############################################ # # LoongArch64 specific patches @@ -1350,6 +1351,7 @@ pushd %{top_level_dir_name} %patch75 -p1 %patch76 -p1 %patch77 -p1 +%patch78 -p1 popd # openjdk %endif @@ -1961,6 +1963,9 @@ cjc.mainProgram(args) -- the returns from copy_jdk_configs.lua should not affect %changelog +* Thu Dec 12 2024 kuenking111 - 1:17.0.13.11-6 +- Add Fix-a-concurrent-issue-of-program-data-ref-cnt.patch + * Fri Dec 6 2024 kuenking111 - 1:17.0.13.11-5 - modify Add-jbolt-feature.patch - modify Add-specialized-hashmap-version-of-the-long-type.patch -- Gitee