packages/openjdk-17
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
openjdk-17/Fix-a-concurrent-issue-of-program-data-ref-cnt.patch
佛系少年中二 7e283f1796 sync master to openEuler-24.03-LTS
2025-02-25 16:45:21 +08:00

364 lines
14 KiB
Diff
Raw Blame History

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<mtJBooster> {
+ // @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<mtJBooster> {
+ // @see test/hotspot/gtest/jbooster/test_server.cpp
+ friend class Gtest_JBoosterServer;
+
public:
using ClassLoaderTable = ConcurrentHashMap<ClassLoaderKey, ClassLoaderData*, mtJBooster>;
@@ -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<K, V, F, Events>::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<K, V, F, Events>::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 <typename T>
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<String> 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<Process> 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<String> 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
Reference in New Issue View Git Blame Copy Permalink