diff options
| -rw-r--r-- | openjdkjvmti/OpenjdkJvmTi.cc | 61 | ||||
| -rw-r--r-- | openjdkjvmti/deopt_manager.cc | 4 | ||||
| -rw-r--r-- | openjdkjvmti/events.cc | 5 | ||||
| -rw-r--r-- | test/1944-sudden-exit/expected.txt | 2 | ||||
| -rw-r--r-- | test/1944-sudden-exit/info.txt | 5 | ||||
| -rwxr-xr-x | test/1944-sudden-exit/run | 18 | ||||
| -rw-r--r-- | test/1944-sudden-exit/src/Main.java | 21 | ||||
| -rw-r--r-- | test/1944-sudden-exit/src/art/Test1944.java | 69 | ||||
| -rw-r--r-- | test/1944-sudden-exit/src/art/Trace.java | 68 | ||||
| -rw-r--r-- | test/1944-sudden-exit/sudden_exit.cc | 32 | ||||
| -rw-r--r-- | test/Android.bp | 1 | ||||
| -rw-r--r-- | tools/external_oj_libjdwp_art_failures.txt | 6 | ||||
| -rw-r--r-- | tools/prebuilt_libjdwp_art_failures.txt | 6 |
13 files changed, 258 insertions, 40 deletions
diff --git a/openjdkjvmti/OpenjdkJvmTi.cc b/openjdkjvmti/OpenjdkJvmTi.cc index a0c7f40b6f..ef5151990c 100644 --- a/openjdkjvmti/OpenjdkJvmTi.cc +++ b/openjdkjvmti/OpenjdkJvmTi.cc @@ -73,8 +73,10 @@ namespace openjdkjvmti { -EventHandler gEventHandler; -DeoptManager gDeoptManager; +// NB These are heap allocated to avoid the static destructors being run if an agent calls exit(3). +// These should never be null. +EventHandler* gEventHandler; +DeoptManager* gDeoptManager; #define ENSURE_NON_NULL(n) \ do { \ @@ -776,7 +778,7 @@ class JvmtiFunctions { ENSURE_HAS_CAP(env, can_retransform_classes); std::string error_msg; jvmtiError res = Transformer::RetransformClasses(ArtJvmTiEnv::AsArtJvmTiEnv(env), - &gEventHandler, + gEventHandler, art::Runtime::Current(), art::Thread::Current(), class_count, @@ -795,7 +797,7 @@ class JvmtiFunctions { ENSURE_HAS_CAP(env, can_redefine_classes); std::string error_msg; jvmtiError res = Redefiner::RedefineClasses(ArtJvmTiEnv::AsArtJvmTiEnv(env), - &gEventHandler, + gEventHandler, art::Runtime::Current(), art::Thread::Current(), class_count, @@ -1061,7 +1063,10 @@ class JvmtiFunctions { } ArtJvmTiEnv* art_env = ArtJvmTiEnv::AsArtJvmTiEnv(env); - return gEventHandler.SetEvent(art_env, art_thread, GetArtJvmtiEvent(art_env, event_type), mode); + return gEventHandler->SetEvent(art_env, + art_thread, + GetArtJvmtiEvent(art_env, event_type), + mode); } static jvmtiError GenerateEvents(jvmtiEnv* env, @@ -1095,7 +1100,7 @@ class JvmtiFunctions { return ExtensionUtil::SetExtensionEventCallback(env, extension_event_index, callback, - &gEventHandler); + gEventHandler); } #define FOR_ALL_CAPABILITIES(FUN) \ @@ -1186,9 +1191,9 @@ class JvmtiFunctions { FOR_ALL_CAPABILITIES(ADD_CAPABILITY); #undef ADD_CAPABILITY - gEventHandler.HandleChangedCapabilities(ArtJvmTiEnv::AsArtJvmTiEnv(env), - changed, - /*added*/true); + gEventHandler->HandleChangedCapabilities(ArtJvmTiEnv::AsArtJvmTiEnv(env), + changed, + /*added*/true); return ret; } @@ -1210,9 +1215,9 @@ class JvmtiFunctions { FOR_ALL_CAPABILITIES(DEL_CAPABILITY); #undef DEL_CAPABILITY - gEventHandler.HandleChangedCapabilities(ArtJvmTiEnv::AsArtJvmTiEnv(env), - changed, - /*added*/false); + gEventHandler->HandleChangedCapabilities(ArtJvmTiEnv::AsArtJvmTiEnv(env), + changed, + /*added*/false); return OK; } @@ -1302,7 +1307,7 @@ class JvmtiFunctions { static jvmtiError DisposeEnvironment(jvmtiEnv* env) { ENSURE_VALID_ENV(env); ArtJvmTiEnv* tienv = ArtJvmTiEnv::AsArtJvmTiEnv(env); - gEventHandler.RemoveArtJvmTiEnv(tienv); + gEventHandler->RemoveArtJvmTiEnv(tienv); art::Runtime::Current()->RemoveSystemWeakHolder(tienv->object_tag_table.get()); ThreadUtil::RemoveEnvironment(tienv); delete tienv; @@ -1490,10 +1495,10 @@ ArtJvmTiEnv::ArtJvmTiEnv(art::JavaVMExt* runtime, EventHandler* event_handler, j // Creates a jvmtiEnv and returns it with the art::ti::Env that is associated with it. new_art_ti // is a pointer to the uninitialized memory for an art::ti::Env. static void CreateArtJvmTiEnv(art::JavaVMExt* vm, jint version, /*out*/void** new_jvmtiEnv) { - struct ArtJvmTiEnv* env = new ArtJvmTiEnv(vm, &gEventHandler, version); + struct ArtJvmTiEnv* env = new ArtJvmTiEnv(vm, gEventHandler, version); *new_jvmtiEnv = env; - gEventHandler.RegisterArtJvmTiEnv(env); + gEventHandler->RegisterArtJvmTiEnv(env); art::Runtime::Current()->AddSystemWeakHolder( ArtJvmTiEnv::AsArtJvmTiEnv(env)->object_tag_table.get()); @@ -1522,17 +1527,20 @@ static jint GetEnvHandler(art::JavaVMExt* vm, /*out*/void** env, jint version) { extern "C" bool ArtPlugin_Initialize() { art::Runtime* runtime = art::Runtime::Current(); - gDeoptManager.Setup(); + gDeoptManager = new DeoptManager; + gEventHandler = new EventHandler; + + gDeoptManager->Setup(); if (runtime->IsStarted()) { PhaseUtil::SetToLive(); } else { PhaseUtil::SetToOnLoad(); } - PhaseUtil::Register(&gEventHandler); - ThreadUtil::Register(&gEventHandler); - ClassUtil::Register(&gEventHandler); - DumpUtil::Register(&gEventHandler); - MethodUtil::Register(&gEventHandler); + PhaseUtil::Register(gEventHandler); + ThreadUtil::Register(gEventHandler); + ClassUtil::Register(gEventHandler); + DumpUtil::Register(gEventHandler); + MethodUtil::Register(gEventHandler); SearchUtil::Register(); HeapUtil::Register(); Transformer::Setup(); @@ -1540,7 +1548,7 @@ extern "C" bool ArtPlugin_Initialize() { { // Make sure we can deopt anything we need to. art::ScopedObjectAccess soa(art::Thread::Current()); - gDeoptManager.FinishSetup(); + gDeoptManager->FinishSetup(); } runtime->GetJavaVM()->AddEnvironmentHook(GetEnvHandler); @@ -1549,8 +1557,8 @@ extern "C" bool ArtPlugin_Initialize() { } extern "C" bool ArtPlugin_Deinitialize() { - gEventHandler.Shutdown(); - gDeoptManager.Shutdown(); + gEventHandler->Shutdown(); + gDeoptManager->Shutdown(); PhaseUtil::Unregister(); ThreadUtil::Unregister(); ClassUtil::Unregister(); @@ -1559,6 +1567,11 @@ extern "C" bool ArtPlugin_Deinitialize() { SearchUtil::Unregister(); HeapUtil::Unregister(); + // TODO It would be good to delete the gEventHandler and gDeoptManager here but we cannot since + // daemon threads might be suspended and we want to make sure that even if they wake up briefly + // they won't hit deallocated memory. By this point none of the functions will do anything since + // they have already shutdown. + return true; } diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc index 9e11a25e58..36998ee976 100644 --- a/openjdkjvmti/deopt_manager.cc +++ b/openjdkjvmti/deopt_manager.cc @@ -343,9 +343,9 @@ void DeoptManager::DeoptimizeThread(art::Thread* target) { art::Runtime::Current()->GetInstrumentation()->InstrumentThreadStack(target); } -extern DeoptManager gDeoptManager; +extern DeoptManager* gDeoptManager; DeoptManager* DeoptManager::Get() { - return &gDeoptManager; + return gDeoptManager; } } // namespace openjdkjvmti diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc index 62b73c08c0..8b40a7e072 100644 --- a/openjdkjvmti/events.cc +++ b/openjdkjvmti/events.cc @@ -1186,8 +1186,9 @@ void EventHandler::Shutdown() { art::Runtime::Current()->GetInstrumentation()->RemoveListener(method_trace_listener_.get(), ~0); } -EventHandler::EventHandler() : envs_lock_("JVMTI Environment List Lock", - art::LockLevel::kTopLockLevel) { +EventHandler::EventHandler() + : envs_lock_("JVMTI Environment List Lock", art::LockLevel::kTopLockLevel), + frame_pop_enabled(false) { alloc_listener_.reset(new JvmtiAllocationListener(this)); ddm_listener_.reset(new JvmtiDdmChunkListener(this)); gc_pause_listener_.reset(new JvmtiGcPauseListener(this)); diff --git a/test/1944-sudden-exit/expected.txt b/test/1944-sudden-exit/expected.txt new file mode 100644 index 0000000000..98f8511769 --- /dev/null +++ b/test/1944-sudden-exit/expected.txt @@ -0,0 +1,2 @@ +All threads started +Exiting suddenly diff --git a/test/1944-sudden-exit/info.txt b/test/1944-sudden-exit/info.txt new file mode 100644 index 0000000000..d575ce5864 --- /dev/null +++ b/test/1944-sudden-exit/info.txt @@ -0,0 +1,5 @@ +Test to make sure the runtime will not crash if an agent calls exit(3) while +other threads are performing operations. + +In this case we have multiple threads all performing single stepping when we +call exit(3). diff --git a/test/1944-sudden-exit/run b/test/1944-sudden-exit/run new file mode 100755 index 0000000000..51875a7e86 --- /dev/null +++ b/test/1944-sudden-exit/run @@ -0,0 +1,18 @@ +#!/bin/bash +# +# Copyright 2017 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Ask for stack traces to be dumped to a file rather than to stdout. +./default-run "$@" --jvmti diff --git a/test/1944-sudden-exit/src/Main.java b/test/1944-sudden-exit/src/Main.java new file mode 100644 index 0000000000..1644c6ef0c --- /dev/null +++ b/test/1944-sudden-exit/src/Main.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + public static void main(String[] args) throws Exception { + art.Test1944.run(); + } +} diff --git a/test/1944-sudden-exit/src/art/Test1944.java b/test/1944-sudden-exit/src/art/Test1944.java new file mode 100644 index 0000000000..36cbb2b390 --- /dev/null +++ b/test/1944-sudden-exit/src/art/Test1944.java @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package art; + +import java.util.Arrays; +import java.lang.reflect.Executable; +import java.lang.reflect.Method; +import java.util.concurrent.Semaphore; + +public class Test1944 { + // Just calculate fib forever. + public static void fib(Semaphore started) { + started.release(); + long a = 1; + long b = 1; + while (true) { + long c = a + b; + a = b; + b = c; + } + } + + // Don't bother actually doing anything. + public static void notifySingleStep(Thread thr, Executable e, long loc) { } + + public static native void exitNow(); + + private static int num_threads = 10; + + public static void run() throws Exception { + final Semaphore started = new Semaphore(-(num_threads - 1)); + + Trace.enableSingleStepTracing(Test1944.class, + Test1944.class.getDeclaredMethod( + "notifySingleStep", Thread.class, Executable.class, Long.TYPE), + null); + + Thread[] threads = new Thread[num_threads]; + for (int i = 0; i < num_threads; i++) { + threads[i] = new Thread(() -> { fib(started); }); + // Make half daemons. + threads[i].setDaemon(i % 2 == 0); + threads[i].start(); + } + // Wait for all threads to start. + started.acquire(); + System.out.println("All threads started"); + // sleep a little + Thread.sleep(10); + // Die. + System.out.println("Exiting suddenly"); + exitNow(); + System.out.println("FAILED: Should not reach here!"); + } +} diff --git a/test/1944-sudden-exit/src/art/Trace.java b/test/1944-sudden-exit/src/art/Trace.java new file mode 100644 index 0000000000..8999bb1368 --- /dev/null +++ b/test/1944-sudden-exit/src/art/Trace.java @@ -0,0 +1,68 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package art; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; + +public class Trace { + public static native void enableTracing(Class<?> methodClass, + Method entryMethod, + Method exitMethod, + Method fieldAccess, + Method fieldModify, + Method singleStep, + Thread thr); + public static native void disableTracing(Thread thr); + + public static void enableFieldTracing(Class<?> methodClass, + Method fieldAccess, + Method fieldModify, + Thread thr) { + enableTracing(methodClass, null, null, fieldAccess, fieldModify, null, thr); + } + + public static void enableMethodTracing(Class<?> methodClass, + Method entryMethod, + Method exitMethod, + Thread thr) { + enableTracing(methodClass, entryMethod, exitMethod, null, null, null, thr); + } + + public static void enableSingleStepTracing(Class<?> methodClass, + Method singleStep, + Thread thr) { + enableTracing(methodClass, null, null, null, null, singleStep, thr); + } + + public static native void watchFieldAccess(Field f); + public static native void watchFieldModification(Field f); + public static native void watchAllFieldAccesses(); + public static native void watchAllFieldModifications(); + + // the names, arguments, and even line numbers of these functions are embedded in the tests so we + // need to add to the bottom and not modify old ones to maintain compat. + public static native void enableTracing2(Class<?> methodClass, + Method entryMethod, + Method exitMethod, + Method fieldAccess, + Method fieldModify, + Method singleStep, + Method ThreadStart, + Method ThreadEnd, + Thread thr); +} diff --git a/test/1944-sudden-exit/sudden_exit.cc b/test/1944-sudden-exit/sudden_exit.cc new file mode 100644 index 0000000000..2f5ce9b959 --- /dev/null +++ b/test/1944-sudden-exit/sudden_exit.cc @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <stdlib.h> +#include "jni.h" + +namespace art { +namespace Test1944SuddenExit { + +extern "C" JNIEXPORT void JNICALL Java_art_Test1944_exitNow(JNIEnv*, jclass) + __attribute__((noreturn)); + +extern "C" JNIEXPORT void JNICALL Java_art_Test1944_exitNow(JNIEnv*, jclass) { + exit(0); +} + +} // namespace Test1944SuddenExit +} // namespace art + diff --git a/test/Android.bp b/test/Android.bp index 72e8eee95a..98e79ad125 100644 --- a/test/Android.bp +++ b/test/Android.bp @@ -294,6 +294,7 @@ art_cc_defaults { "936-search-onload/search_onload.cc", "983-source-transform-verify/source_transform.cc", "1940-ddms-ext/ddm_ext.cc", + "1944-sudden-exit/sudden_exit.cc", ], } diff --git a/tools/external_oj_libjdwp_art_failures.txt b/tools/external_oj_libjdwp_art_failures.txt index 828c0aac0f..ba764dfc99 100644 --- a/tools/external_oj_libjdwp_art_failures.txt +++ b/tools/external_oj_libjdwp_art_failures.txt @@ -66,12 +66,6 @@ "org.apache.harmony.jpda.tests.jdwp.MultiSession.EnableCollectionTest#testEnableCollection001" ] }, { - description: "Test crashes", - result: EXEC_FAILED, - bug: 69591477, - name: "org.apache.harmony.jpda.tests.jdwp.VirtualMachine.ExitTest#testExit001" -}, -{ description: "Test times out on fugu-debug", result: EXEC_FAILED, bug: 70459916, diff --git a/tools/prebuilt_libjdwp_art_failures.txt b/tools/prebuilt_libjdwp_art_failures.txt index a9d268de0a..5251daf699 100644 --- a/tools/prebuilt_libjdwp_art_failures.txt +++ b/tools/prebuilt_libjdwp_art_failures.txt @@ -119,11 +119,5 @@ bug: 70958370, names: [ "org.apache.harmony.jpda.tests.jdwp.ObjectReference.EnableCollectionTest#testEnableCollection001", "org.apache.harmony.jpda.tests.jdwp.MultiSession.EnableCollectionTest#testEnableCollection001" ] -}, -{ - description: "Test crashes", - result: EXEC_FAILED, - bug: 69591477, - name: "org.apache.harmony.jpda.tests.jdwp.VirtualMachine.ExitTest#testExit001" } ] |