Revert "Prevent races with GC when transferring objects between threads"
This reverts commit 7585b91bfc77b8e4b821ccfa716fa86e46455275.
Reason for revert: Seems to be causing fatal error:
Checkpoint flag set without pending checkpoint
Change-Id: I98ea653e629d73e854907115583afed3ed5ac68e
Test: None.
Bug: 67838964
diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc
index fccc0b1..41679da 100644
--- a/openjdkjvmti/ti_method.cc
+++ b/openjdkjvmti/ti_method.cc
@@ -788,7 +788,7 @@
}
GetLocalVariableClosure c(self, depth, slot, type, val);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(target, &c)) {
+ if (!target->RequestSynchronousCheckpoint(&c)) {
return ERR(THREAD_NOT_ALIVE);
} else {
return c.GetResult();
@@ -917,7 +917,7 @@
}
SetLocalVariableClosure c(self, depth, slot, type, val);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(target, &c)) {
+ if (!target->RequestSynchronousCheckpoint(&c)) {
return ERR(THREAD_NOT_ALIVE);
} else {
return c.GetResult();
@@ -976,7 +976,7 @@
}
GetLocalInstanceClosure c(self, depth, data);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(target, &c)) {
+ if (!target->RequestSynchronousCheckpoint(&c)) {
return ERR(THREAD_NOT_ALIVE);
} else {
return c.GetResult();
diff --git a/openjdkjvmti/ti_monitor.cc b/openjdkjvmti/ti_monitor.cc
index 7db0566..5881f8c 100644
--- a/openjdkjvmti/ti_monitor.cc
+++ b/openjdkjvmti/ti_monitor.cc
@@ -395,7 +395,7 @@
};
GetContendedMonitorClosure closure(self, monitor);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(target, &closure)) {
+ if (!target->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
return OK;
diff --git a/openjdkjvmti/ti_stack.cc b/openjdkjvmti/ti_stack.cc
index b43eaa0..e346e16 100644
--- a/openjdkjvmti/ti_stack.cc
+++ b/openjdkjvmti/ti_stack.cc
@@ -257,7 +257,7 @@
static_cast<size_t>(start_depth),
static_cast<size_t>(max_frame_count));
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(thread, &closure)) {
+ if (!thread->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
*count_ptr = static_cast<jint>(closure.index);
@@ -268,7 +268,7 @@
} else {
GetStackTraceVectorClosure closure(0, 0);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(thread, &closure)) {
+ if (!thread->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
@@ -712,7 +712,7 @@
GetFrameCountClosure closure;
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(thread, &closure)) {
+ if (!thread->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
@@ -802,7 +802,7 @@
GetLocationClosure closure(static_cast<size_t>(depth));
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(thread, &closure)) {
+ if (!thread->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
@@ -923,7 +923,7 @@
if (target != self) {
called_method = true;
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (!ThreadUtil::RequestGCSafeSynchronousCheckpoint(target, &closure)) {
+ if (!target->RequestSynchronousCheckpoint(&closure)) {
return ERR(THREAD_NOT_ALIVE);
}
} else {
diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc
index 6d075a6..99dea54 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -38,9 +38,6 @@
#include "base/mutex.h"
#include "events-inl.h"
#include "gc/system_weak.h"
-#include "gc/collector_type.h"
-#include "gc/gc_cause.h"
-#include "gc/scoped_gc_critical_section.h"
#include "gc_root-inl.h"
#include "jni_internal.h"
#include "mirror/class.h"
@@ -1064,7 +1061,7 @@
};
StopThreadClosure c(exc);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
- if (RequestGCSafeSynchronousCheckpoint(target, &c)) {
+ if (target->RequestSynchronousCheckpoint(&c)) {
return OK;
} else {
// Something went wrong, probably the thread died.
@@ -1087,29 +1084,4 @@
return OK;
}
-class GcCriticalSectionClosure : public art::Closure {
- public:
- explicit GcCriticalSectionClosure(art::Closure* wrapped) : wrapped_(wrapped) {}
-
- void Run(art::Thread* self) OVERRIDE {
- if (art::kIsDebugBuild) {
- art::Locks::thread_list_lock_->AssertNotHeld(art::Thread::Current());
- }
- // This might block as it waits for any in-progress GCs to finish but this is fine since we
- // released the Thread-list-lock prior to calling this in RequestSynchronousCheckpoint.
- art::gc::ScopedGCCriticalSection sgccs(art::Thread::Current(),
- art::gc::kGcCauseDebugger,
- art::gc::kCollectorTypeDebugger);
- wrapped_->Run(self);
- }
-
- private:
- art::Closure* wrapped_;
-};
-
-bool ThreadUtil::RequestGCSafeSynchronousCheckpoint(art::Thread* thr, art::Closure* function) {
- GcCriticalSectionClosure gccsc(function);
- return thr->RequestSynchronousCheckpoint(&gccsc);
-}
-
} // namespace openjdkjvmti
diff --git a/openjdkjvmti/ti_thread.h b/openjdkjvmti/ti_thread.h
index 341bffe..09b4cab 100644
--- a/openjdkjvmti/ti_thread.h
+++ b/openjdkjvmti/ti_thread.h
@@ -42,7 +42,6 @@
class ArtField;
class ScopedObjectAccessAlreadyRunnable;
class Thread;
-class Closure;
} // namespace art
namespace openjdkjvmti {
@@ -134,16 +133,6 @@
REQUIRES(!art::Locks::user_code_suspension_lock_,
!art::Locks::thread_suspend_count_lock_);
- // This will request a synchronous checkpoint in such a way as to prevent gc races if a local
- // variable is taken from one thread's stack and placed in the stack of another thread.
- // RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution. This is
- // due to the fact that Thread::Current() needs to go to sleep to allow the targeted thread to
- // execute the checkpoint for us if it is Runnable.
- static bool RequestGCSafeSynchronousCheckpoint(art::Thread* thr, art::Closure* function)
- REQUIRES_SHARED(art::Locks::mutator_lock_)
- RELEASE(art::Locks::thread_list_lock_)
- REQUIRES(!art::Locks::thread_suspend_count_lock_);
-
private:
// We need to make sure only one thread tries to suspend threads at a time so we can get the
// 'suspend-only-once' behavior the spec requires. Internally, ART considers suspension to be a
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index cdea0e5..0033167 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -553,7 +553,7 @@
: self_(self),
arena_stack_(Runtime::Current()->GetArenaPool()),
allocator_(&arena_stack_),
- reg_types_(can_load_classes, allocator_, allow_thread_suspension),
+ reg_types_(can_load_classes, allocator_),
reg_table_(allocator_),
work_insn_idx_(dex::kDexNoIndex),
dex_method_idx_(dex_method_idx),
diff --git a/runtime/verifier/reg_type_cache.cc b/runtime/verifier/reg_type_cache.cc
index 0029eb9..4ebe151 100644
--- a/runtime/verifier/reg_type_cache.cc
+++ b/runtime/verifier/reg_type_cache.cc
@@ -268,13 +268,12 @@
return *reg_type;
}
-RegTypeCache::RegTypeCache(bool can_load_classes, ScopedArenaAllocator& allocator, bool can_suspend)
+RegTypeCache::RegTypeCache(bool can_load_classes, ScopedArenaAllocator& allocator)
: entries_(allocator.Adapter(kArenaAllocVerifier)),
klass_entries_(allocator.Adapter(kArenaAllocVerifier)),
can_load_classes_(can_load_classes),
allocator_(allocator) {
- DCHECK(can_suspend || !can_load_classes) << "Cannot load classes is suspension is disabled!";
- if (kIsDebugBuild && can_suspend) {
+ if (kIsDebugBuild) {
Thread::Current()->AssertThreadSuspensionIsAllowable(gAborting == 0);
}
// The klass_entries_ array does not have primitives or small constants.
diff --git a/runtime/verifier/reg_type_cache.h b/runtime/verifier/reg_type_cache.h
index cb16b15..74d9e9d 100644
--- a/runtime/verifier/reg_type_cache.h
+++ b/runtime/verifier/reg_type_cache.h
@@ -61,7 +61,7 @@
class RegTypeCache {
public:
- RegTypeCache(bool can_load_classes, ScopedArenaAllocator& allocator, bool can_suspend = true);
+ explicit RegTypeCache(bool can_load_classes, ScopedArenaAllocator& allocator);
~RegTypeCache();
static void Init() REQUIRES_SHARED(Locks::mutator_lock_) {
if (!RegTypeCache::primitive_initialized_) {