Revert "Fix a race when dumping thread list for tracing"
This reverts commit bd10861f33fcc1a1e7ddae3c48eb452a398e74a4.
Reason for revert: Breaks few builds:
https://ci.chromium.org/ui/p/art/builders/ci/host-x86-poison-debug/7874/overview
Change-Id: If82aa3c2f66396cdc08cf95a841ef90ac36bc201
diff --git a/runtime/trace.cc b/runtime/trace.cc
index bad4992..051a600 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -621,11 +621,6 @@
the_trace_->FlushStreamingBuffer(thread);
thread->ResetMethodTraceBuffer();
}
- // Record threads here before resetting the_trace_ to prevent any races between
- // unregistering the thread and resetting the_trace_.
- std::string name;
- thread->GetThreadName(name);
- the_trace->threads_list_.Put(thread->GetTid(), name);
}
}
@@ -1247,17 +1242,28 @@
}
}
+static void DumpThread(Thread* t, void* arg) {
+ std::ostream& os = *reinterpret_cast<std::ostream*>(arg);
+ std::string name;
+ t->GetThreadName(name);
+ // We use only 16 bits to encode thread id. On Android, we don't expect to use more than
+ // 16-bits for a Tid. For 32-bit platforms it is always ensured we use less than 16 bits.
+ // See __check_max_thread_id in bionic for more details. Even on 64-bit the max threads
+ // is currently less than 65536.
+ // TODO(mythria): On host, we know thread ids can be greater than 16 bits. Consider adding
+ // a map similar to method ids.
+ DCHECK(!kIsTargetBuild || t->GetTid() < (1 << 16));
+ os << static_cast<uint16_t>(t->GetTid()) << "\t" << name << "\n";
+}
+
void Trace::DumpThreadList(std::ostream& os) {
- for (const auto& it : threads_list_) {
- // We use only 16 bits to encode thread id. On Android, we don't expect to use more than
- // 16-bits for a Tid. For 32-bit platforms it is always ensured we use less than 16 bits.
- // See __check_max_thread_id in bionic for more details. Even on 64-bit the max threads
- // is currently less than 65536.
- // TODO(mythria): On host, we know thread ids can be greater than 16 bits. Consider adding
- // a map similar to method ids.
- DCHECK(!kIsTargetBuild || it.first < (1 << 16));
+ Thread* self = Thread::Current();
+ for (const auto& it : exited_threads_) {
os << static_cast<uint16_t>(it.first) << "\t" << it.second << "\n";
}
+ Locks::thread_list_lock_->AssertNotHeld(self);
+ MutexLock mu(self, *Locks::thread_list_lock_);
+ Runtime::Current()->GetThreadList()->ForEach(DumpThread, &os);
}
void Trace::StoreExitingThreadInfo(Thread* thread) {
@@ -1265,7 +1271,9 @@
if (the_trace_ != nullptr) {
std::string name;
thread->GetThreadName(name);
- the_trace_->threads_list_.Put(thread->GetTid(), name);
+ // The same thread/tid may be used multiple times. As SafeMap::Put does not allow to override
+ // a previous mapping, use SafeMap::Overwrite.
+ the_trace_->exited_threads_.Overwrite(thread->GetTid(), name);
}
}
diff --git a/runtime/trace.h b/runtime/trace.h
index 5f6f3c2..c3f903c 100644
--- a/runtime/trace.h
+++ b/runtime/trace.h
@@ -395,9 +395,8 @@
// Did we overflow the buffer recording traces?
bool overflow_;
- // Map of thread ids and names. We record the information when the threads are
- // exiting and when the tracing has finished.
- SafeMap<pid_t, std::string> threads_list_;
+ // Map of thread ids and names that have already exited.
+ SafeMap<pid_t, std::string> exited_threads_;
// Sampling profiler sampling interval.
int interval_us_;
diff --git a/test/2246-trace-stream/expected-stdout.txt b/test/2246-trace-stream/expected-stdout.txt
index 90706b5..cb4c0ca 100644
--- a/test/2246-trace-stream/expected-stdout.txt
+++ b/test/2246-trace-stream/expected-stdout.txt
@@ -72,85 +72,3 @@
....<<E main Main callThrowFunction ()V Main.java
...<< main Main doSomeWorkThrow ()V Main.java
...>> main Main$VMDebug $noinline$stopMethodTracing ()V Main.java
-***** non streaming test *******
-.>> main Main main ([Ljava/lang/String;)V Main.java
-..>> main Main testTracing (ZLBaseTraceParser;I)V Main.java
-...>> main Main$VMDebug startMethodTracing (Ljava/lang/String;Ljava/io/FileDescriptor;IIZIZ)V Main.java
-....>> main java.lang.reflect.Method invoke (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; Method.java
-.....>> main dalvik.system.VMDebug startMethodTracing (Ljava/lang/String;Ljava/io/FileDescriptor;IIZIZ)V VMDebug.java
-......>> main dalvik.system.VMDebug startMethodTracingFd (Ljava/lang/String;IIIZIZ)V VMDebug.java
-......<< main dalvik.system.VMDebug startMethodTracingFd (Ljava/lang/String;IIIZIZ)V VMDebug.java
-.....<< main dalvik.system.VMDebug startMethodTracing (Ljava/lang/String;Ljava/io/FileDescriptor;IIZIZ)V VMDebug.java
-....<< main java.lang.reflect.Method invoke (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; Method.java
-...<< main Main$VMDebug startMethodTracing (Ljava/lang/String;Ljava/io/FileDescriptor;IIZIZ)V Main.java
-...>> main java.lang.Thread start ()V Thread.java
-....>> main java.lang.ThreadGroup add (Ljava/lang/Thread;)V ThreadGroup.java
-....<< main java.lang.ThreadGroup add (Ljava/lang/Thread;)V ThreadGroup.java
-....>> main java.lang.Thread nativeCreate (Ljava/lang/Thread;JZ)V Thread.java
-....<< main java.lang.Thread nativeCreate (Ljava/lang/Thread;JZ)V Thread.java
-...<< main java.lang.Thread start ()V Thread.java
-...>> main java.lang.Thread join ()V Thread.java
-....>> main java.lang.Thread join (J)V Thread.java
-.....>> main java.lang.System currentTimeMillis ()J System.java
-.....<< main java.lang.System currentTimeMillis ()J System.java
-.....>> main java.lang.Thread isAlive ()Z Thread.java
-.....<< main java.lang.Thread isAlive ()Z Thread.java
-.....>> main java.lang.Object wait (J)V Object.java
-......>> main java.lang.Object wait (JI)V Object.java
-.>> TestThread2246 java.lang.Thread run ()V Thread.java
-..>> TestThread2246 Main$$ExternalSyntheticLambda0 run ()V D8$$SyntheticClass
-...>> TestThread2246 Main lambda$testTracing$0 ()V Main.java
-....>> TestThread2246 Main <init> ()V Main.java
-.....>> TestThread2246 java.lang.Object <init> ()V Object.java
-.....<< TestThread2246 java.lang.Object <init> ()V Object.java
-....<< TestThread2246 Main <init> ()V Main.java
-....>> TestThread2246 Main $noinline$doSomeWork ()V Main.java
-.....>> TestThread2246 Main callOuterFunction ()V Main.java
-......>> TestThread2246 Main callLeafFunction ()V Main.java
-......<< TestThread2246 Main callLeafFunction ()V Main.java
-.....<< TestThread2246 Main callOuterFunction ()V Main.java
-.....>> TestThread2246 Main callLeafFunction ()V Main.java
-.....<< TestThread2246 Main callLeafFunction ()V Main.java
-....<< TestThread2246 Main $noinline$doSomeWork ()V Main.java
-...<< TestThread2246 Main lambda$testTracing$0 ()V Main.java
-..<< TestThread2246 Main$$ExternalSyntheticLambda0 run ()V D8$$SyntheticClass
-.<< TestThread2246 java.lang.Thread run ()V Thread.java
-.>> TestThread2246 java.lang.ThreadGroup threadTerminated (Ljava/lang/Thread;)V ThreadGroup.java
-..>> TestThread2246 java.lang.ThreadGroup remove (Ljava/lang/Thread;)V ThreadGroup.java
-...>> TestThread2246 java.lang.System arraycopy (Ljava/lang/Object;ILjava/lang/Object;II)V System.java
-...<< TestThread2246 java.lang.System arraycopy (Ljava/lang/Object;ILjava/lang/Object;II)V System.java
-..<< TestThread2246 java.lang.ThreadGroup remove (Ljava/lang/Thread;)V ThreadGroup.java
-.<< TestThread2246 java.lang.ThreadGroup threadTerminated (Ljava/lang/Thread;)V ThreadGroup.java
-......<< main java.lang.Object wait (JI)V Object.java
-.....<< main java.lang.Object wait (J)V Object.java
-.....>> main java.lang.Thread isAlive ()Z Thread.java
-.....<< main java.lang.Thread isAlive ()Z Thread.java
-....<< main java.lang.Thread join (J)V Thread.java
-...<< main java.lang.Thread join ()V Thread.java
-...>> main Main $noinline$doSomeWork ()V Main.java
-....>> main Main callOuterFunction ()V Main.java
-.....>> main Main callLeafFunction ()V Main.java
-.....<< main Main callLeafFunction ()V Main.java
-....<< main Main callOuterFunction ()V Main.java
-....>> main Main callLeafFunction ()V Main.java
-....<< main Main callLeafFunction ()V Main.java
-...<< main Main $noinline$doSomeWork ()V Main.java
-...>> main Main doSomeWorkThrow ()V Main.java
-....>> main Main callThrowFunction ()V Main.java
-.....>> main java.lang.Exception <init> (Ljava/lang/String;)V Exception.java
-......>> main java.lang.Throwable <init> (Ljava/lang/String;)V Throwable.java
-.......>> main java.lang.Object <init> ()V Object.java
-.......<< main java.lang.Object <init> ()V Object.java
-.......>> main java.util.Collections emptyList ()Ljava/util/List; Collections.java
-.......<< main java.util.Collections emptyList ()Ljava/util/List; Collections.java
-.......>> main java.lang.Throwable fillInStackTrace ()Ljava/lang/Throwable; Throwable.java
-........>> main java.lang.Throwable nativeFillInStackTrace ()Ljava/lang/Object; Throwable.java
-........<< main java.lang.Throwable nativeFillInStackTrace ()Ljava/lang/Object; Throwable.java
-.......<< main java.lang.Throwable fillInStackTrace ()Ljava/lang/Throwable; Throwable.java
-......<< main java.lang.Throwable <init> (Ljava/lang/String;)V Throwable.java
-.....<< main java.lang.Exception <init> (Ljava/lang/String;)V Exception.java
-....<<E main Main callThrowFunction ()V Main.java
-...<< main Main doSomeWorkThrow ()V Main.java
-...>> main Main$VMDebug $noinline$stopMethodTracing ()V Main.java
-....>> main java.lang.reflect.Method invoke (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; Method.java
-.....>> main dalvik.system.VMDebug stopMethodTracing ()V VMDebug.java
diff --git a/test/2246-trace-stream/src/Main.java b/test/2246-trace-stream/src/Main.java
index daada7d..942061f 100644
--- a/test/2246-trace-stream/src/Main.java
+++ b/test/2246-trace-stream/src/Main.java
@@ -36,9 +36,10 @@
testTracing(
/* streaming=*/true, stream_parser, BaseTraceParser.STREAMING_DUAL_CLOCK_VERSION);
- System.out.println("***** non streaming test *******");
- NonStreamTraceParser non_stream_parser = new NonStreamTraceParser();
- testTracing(/* streaming=*/false, non_stream_parser, BaseTraceParser.DUAL_CLOCK_VERSION);
+ // TODO(mythria): Enable after fixing failures on the bots.
+ // System.out.println("***** non streaming test *******");
+ // NonStreamTraceParser non_stream_parser = new NonStreamTraceParser();
+ // testTracing(/* streaming=*/false, non_stream_parser, BaseTraceParser.DUAL_CLOCK_VERSION);
}
public static void testTracing(boolean streaming, BaseTraceParser parser, int expected_version)
diff --git a/test/2246-trace-stream/src/NonStreamTraceParser.java b/test/2246-trace-stream/src/NonStreamTraceParser.java
index db0450f..3219591 100644
--- a/test/2246-trace-stream/src/NonStreamTraceParser.java
+++ b/test/2246-trace-stream/src/NonStreamTraceParser.java
@@ -52,7 +52,7 @@
line = readLine();
while (!line.startsWith(START_SECTION_ID)) {
- String[] threadInfo = line.split("\t", 2);
+ String[] threadInfo = line.split("\t");
threadIdMap.put(Integer.decode(threadInfo[0]), threadInfo[1]);
line = readLine();
}