summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--runtime/runtime.cc24
-rw-r--r--runtime/runtime.h9
-rw-r--r--runtime/thread.cc22
-rw-r--r--runtime/thread.h12
-rw-r--r--runtime/thread_list.cc4
-rw-r--r--runtime/thread_list.h2
-rw-r--r--runtime/thread_pool.cc11
-rw-r--r--tools/external_oj_libjdwp_art_no_read_barrier_failures.txt10
8 files changed, 56 insertions, 38 deletions
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 85249de36c..46429e8156 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -338,10 +338,16 @@ Runtime::~Runtime() {
// In this case we will just try again without allocating a peer so that shutdown can continue.
// Very few things are actually capable of distinguishing between the peer & peerless states so
// this should be fine.
+ // Running callbacks is prone to deadlocks in libjdwp tests that need an event handler lock to
+ // process any event. We also need to enter a GCCriticalSection when processing certain events
+ // (for ex: removing the last breakpoint). These two restrictions together make the tear down
+ // of the jdwp tests deadlock prone if we fail to finish Thread::Attach callback.
+ // (TODO:b/251163712) Remove this once we update deopt manager to not use GCCriticalSection.
bool thread_attached = AttachCurrentThread("Shutdown thread",
/* as_daemon= */ false,
GetSystemThreadGroup(),
- /* create_peer= */ IsStarted());
+ /* create_peer= */ IsStarted(),
+ /* should_run_callbacks= */ false);
if (UNLIKELY(!thread_attached)) {
LOG(WARNING) << "Failed to attach shutdown thread. Trying again without a peer.";
CHECK(AttachCurrentThread("Shutdown thread (no java peer)",
@@ -441,7 +447,7 @@ Runtime::~Runtime() {
}
if (attach_shutdown_thread) {
- DetachCurrentThread();
+ DetachCurrentThread(/* should_run_callbacks= */ false);
self = nullptr;
}
@@ -1829,7 +1835,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
// ClassLinker needs an attached thread, but we can't fully attach a thread without creating
// objects. We can't supply a thread group yet; it will be fixed later. Since we are the main
// thread, we do not get a java peer.
- Thread* self = Thread::Attach("main", false, nullptr, false);
+ Thread* self = Thread::Attach("main", false, nullptr, false, /* should_run_callbacks= */ true);
CHECK_EQ(self->GetThreadId(), ThreadList::kMainThreadId);
CHECK(self != nullptr);
@@ -2430,9 +2436,13 @@ void Runtime::BlockSignals() {
}
bool Runtime::AttachCurrentThread(const char* thread_name, bool as_daemon, jobject thread_group,
- bool create_peer) {
+ bool create_peer, bool should_run_callbacks) {
ScopedTrace trace(__FUNCTION__);
- Thread* self = Thread::Attach(thread_name, as_daemon, thread_group, create_peer);
+ Thread* self = Thread::Attach(thread_name,
+ as_daemon,
+ thread_group,
+ create_peer,
+ should_run_callbacks);
// Run ThreadGroup.add to notify the group that this thread is now started.
if (self != nullptr && create_peer && !IsAotCompiler()) {
ScopedObjectAccess soa(self);
@@ -2441,7 +2451,7 @@ bool Runtime::AttachCurrentThread(const char* thread_name, bool as_daemon, jobje
return self != nullptr;
}
-void Runtime::DetachCurrentThread() {
+void Runtime::DetachCurrentThread(bool should_run_callbacks) {
ScopedTrace trace(__FUNCTION__);
Thread* self = Thread::Current();
if (self == nullptr) {
@@ -2450,7 +2460,7 @@ void Runtime::DetachCurrentThread() {
if (self->HasManagedStack()) {
LOG(FATAL) << *Thread::Current() << " attempting to detach while still running code";
}
- thread_list_->Unregister(self);
+ thread_list_->Unregister(self, should_run_callbacks);
}
mirror::Throwable* Runtime::GetPreAllocatedOutOfMemoryErrorWhenThrowingException() {
diff --git a/runtime/runtime.h b/runtime/runtime.h
index c155489eb1..10ee4aed90 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -291,13 +291,16 @@ class Runtime {
jobject GetSystemClassLoader() const;
// Attaches the calling native thread to the runtime.
- bool AttachCurrentThread(const char* thread_name, bool as_daemon, jobject thread_group,
- bool create_peer);
+ bool AttachCurrentThread(const char* thread_name,
+ bool as_daemon,
+ jobject thread_group,
+ bool create_peer,
+ bool should_run_callbacks = true);
void CallExitHook(jint status);
// Detaches the current native thread from the runtime.
- void DetachCurrentThread() REQUIRES(!Locks::mutator_lock_);
+ void DetachCurrentThread(bool should_run_callbacks = true) REQUIRES(!Locks::mutator_lock_);
void DumpDeoptimizations(std::ostream& os);
void DumpForSigQuit(std::ostream& os);
diff --git a/runtime/thread.cc b/runtime/thread.cc
index ad6b8c6118..58f1dc1d17 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -669,7 +669,7 @@ void* Thread::CreateCallback(void* arg) {
InvokeVirtualOrInterfaceWithJValues(soa, ref.get(), mid, nullptr);
}
// Detach and delete self.
- Runtime::Current()->GetThreadList()->Unregister(self);
+ Runtime::Current()->GetThreadList()->Unregister(self, /* should_run_callbacks= */ true);
return nullptr;
}
@@ -989,7 +989,10 @@ bool Thread::Init(ThreadList* thread_list, JavaVMExt* java_vm, JNIEnvExt* jni_en
}
template <typename PeerAction>
-Thread* Thread::Attach(const char* thread_name, bool as_daemon, PeerAction peer_action) {
+Thread* Thread::Attach(const char* thread_name,
+ bool as_daemon,
+ PeerAction peer_action,
+ bool should_run_callbacks) {
Runtime* runtime = Runtime::Current();
ScopedTrace trace("Thread::Attach");
if (runtime == nullptr) {
@@ -1024,7 +1027,7 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, PeerAction peer_
// Run the action that is acting on the peer.
if (!peer_action(self)) {
- runtime->GetThreadList()->Unregister(self);
+ runtime->GetThreadList()->Unregister(self, should_run_callbacks);
// Unregister deletes self, no need to do this here.
return nullptr;
}
@@ -1039,7 +1042,7 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, PeerAction peer_
self->Dump(LOG_STREAM(INFO));
}
- {
+ if (should_run_callbacks) {
ScopedObjectAccess soa(self);
runtime->GetRuntimeCallbacks()->ThreadStart(self);
}
@@ -1050,7 +1053,8 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, PeerAction peer_
Thread* Thread::Attach(const char* thread_name,
bool as_daemon,
jobject thread_group,
- bool create_peer) {
+ bool create_peer,
+ bool should_run_callbacks) {
auto create_peer_action = [&](Thread* self) {
// If we're the main thread, ClassLinker won't be created until after we're attached,
// so that thread needs a two-stage attach. Regular threads don't need this hack.
@@ -1083,7 +1087,7 @@ Thread* Thread::Attach(const char* thread_name,
}
return true;
};
- return Attach(thread_name, as_daemon, create_peer_action);
+ return Attach(thread_name, as_daemon, create_peer_action, should_run_callbacks);
}
Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_peer) {
@@ -1099,7 +1103,7 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_p
reinterpret_cast64<jlong>(self));
return true;
};
- return Attach(thread_name, as_daemon, set_peer_action);
+ return Attach(thread_name, as_daemon, set_peer_action, /* should_run_callbacks= */ true);
}
void Thread::CreatePeer(const char* name, bool as_daemon, jobject thread_group) {
@@ -2540,7 +2544,7 @@ class MonitorExitVisitor : public SingleRootVisitor {
Thread* const self_;
};
-void Thread::Destroy() {
+void Thread::Destroy(bool should_run_callbacks) {
Thread* self = this;
DCHECK_EQ(self, Thread::Current());
@@ -2569,7 +2573,7 @@ void Thread::Destroy() {
HandleUncaughtExceptions(soa);
RemoveFromThreadGroup(soa);
Runtime* runtime = Runtime::Current();
- if (runtime != nullptr) {
+ if (runtime != nullptr && should_run_callbacks) {
runtime->GetRuntimeCallbacks()->ThreadDeath(self);
}
diff --git a/runtime/thread.h b/runtime/thread.h
index dda4c086f4..bbffaf94b0 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -232,8 +232,11 @@ class Thread {
// Attaches the calling native thread to the runtime, returning the new native peer.
// Used to implement JNI AttachCurrentThread and AttachCurrentThreadAsDaemon calls.
- static Thread* Attach(const char* thread_name, bool as_daemon, jobject thread_group,
- bool create_peer);
+ static Thread* Attach(const char* thread_name,
+ bool as_daemon,
+ jobject thread_group,
+ bool create_peer,
+ bool should_run_callbacks);
// Attaches the calling native thread to the runtime, returning the new native peer.
static Thread* Attach(const char* thread_name, bool as_daemon, jobject thread_peer);
@@ -1466,7 +1469,7 @@ class Thread {
private:
explicit Thread(bool daemon);
~Thread() REQUIRES(!Locks::mutator_lock_, !Locks::thread_suspend_count_lock_);
- void Destroy();
+ void Destroy(bool should_run_callbacks);
// Deletes and clears the tlsPtr_.jpeer field. Done in a way so that both it and opeer cannot be
// observed to be set at the same time by instrumentation.
@@ -1477,7 +1480,8 @@ class Thread {
template <typename PeerAction>
static Thread* Attach(const char* thread_name,
bool as_daemon,
- PeerAction p);
+ PeerAction p,
+ bool should_run_callbacks);
void CreatePeer(const char* name, bool as_daemon, jobject thread_group);
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index f36e922fa6..d2614a69b5 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -1298,7 +1298,7 @@ void ThreadList::Register(Thread* self) {
}
}
-void ThreadList::Unregister(Thread* self) {
+void ThreadList::Unregister(Thread* self, bool should_run_callbacks) {
DCHECK_EQ(self, Thread::Current());
CHECK_NE(self->GetState(), ThreadState::kRunnable);
Locks::mutator_lock_->AssertNotHeld(self);
@@ -1319,7 +1319,7 @@ void ThreadList::Unregister(Thread* self) {
// causes the threads to join. It is important to do this after incrementing unregistering_count_
// since we want the runtime to wait for the daemon threads to exit before deleting the thread
// list.
- self->Destroy();
+ self->Destroy(should_run_callbacks);
// If tracing, remember thread id and name before thread exits.
Trace::StoreExitingThreadInfo(self);
diff --git a/runtime/thread_list.h b/runtime/thread_list.h
index 17a53fa58d..c1ffe9e59d 100644
--- a/runtime/thread_list.h
+++ b/runtime/thread_list.h
@@ -153,7 +153,7 @@ class ThreadList {
REQUIRES(!Locks::mutator_lock_,
!Locks::thread_list_lock_,
!Locks::thread_suspend_count_lock_);
- void Unregister(Thread* self)
+ void Unregister(Thread* self, bool should_run_callbacks)
REQUIRES(!Locks::mutator_lock_,
!Locks::thread_list_lock_,
!Locks::thread_suspend_count_lock_);
diff --git a/runtime/thread_pool.cc b/runtime/thread_pool.cc
index b361d16d95..92ac8452ba 100644
--- a/runtime/thread_pool.cc
+++ b/runtime/thread_pool.cc
@@ -119,6 +119,12 @@ void ThreadPoolWorker::Run() {
void* ThreadPoolWorker::Callback(void* arg) {
ThreadPoolWorker* worker = reinterpret_cast<ThreadPoolWorker*>(arg);
Runtime* runtime = Runtime::Current();
+ // Don't run callbacks for ThreadPoolWorkers. These are created for JITThreadPool and
+ // HeapThreadPool and are purely internal threads of the runtime and we don't need to run
+ // callbacks for the thread attach / detach listeners.
+ // (b/251163712) Calling callbacks for heap thread pool workers causes deadlocks in some libjdwp
+ // tests. Deadlocks happen when a GC thread is attached while libjdwp holds the event handler
+ // lock for an event that triggers an entrypoint update from deopt manager.
CHECK(runtime->AttachCurrentThread(
worker->name_.c_str(),
true,
@@ -129,13 +135,14 @@ void* ThreadPoolWorker::Callback(void* arg) {
// rely on being able to (for example) wait for all threads to finish some task. If debuggers
// are suspending these threads that might not be possible.
worker->thread_pool_->create_peers_ ? runtime->GetSystemThreadGroup() : nullptr,
- worker->thread_pool_->create_peers_));
+ worker->thread_pool_->create_peers_,
+ /* should_run_callbacks= */ false));
worker->thread_ = Thread::Current();
// Mark thread pool workers as runtime-threads.
worker->thread_->SetIsRuntimeThread(true);
// Do work until its time to shut down.
worker->Run();
- runtime->DetachCurrentThread();
+ runtime->DetachCurrentThread(/* should_run_callbacks= */ false);
return nullptr;
}
diff --git a/tools/external_oj_libjdwp_art_no_read_barrier_failures.txt b/tools/external_oj_libjdwp_art_no_read_barrier_failures.txt
index d60862e999..920b611e43 100644
--- a/tools/external_oj_libjdwp_art_no_read_barrier_failures.txt
+++ b/tools/external_oj_libjdwp_art_no_read_barrier_failures.txt
@@ -6,14 +6,4 @@
* test groups on the chromium buildbot running without read-barrier.
*/
[
-{
- description: "Failing with userfaultfd GC",
- result: EXEC_FAILED,
- bug: 242181443,
- names: [ "org.apache.harmony.jpda.tests.jdwp.ObjectReference_DisableCollectionTest#testDisableCollection001",
- "org.apache.harmony.jpda.tests.jdwp.ObjectReference_DisableCollectionTest#testDisableCollection_invalid",
- "org.apache.harmony.jpda.tests.jdwp.ObjectReference_IsCollectedTest#testIsCollected001",
- "org.apache.harmony.jpda.tests.jdwp.ObjectReference_IsCollectedTest#testIsCollected_invalid",
- "org.apache.harmony.jpda.tests.jdwp.ObjectReference_IsCollectedTest#testIsCollected_null" ]
-}
]
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-21 07:45:09 +0000