Reland "Fix a race when dumping thread list for tracing"
This reverts commit b51bdccb0b44b1aa9295bd3de03dbe98c7fbfa0f.
Reason for revert: Reland after fixing failures.
We might update thread information more than once when thread detach and
stopping the trace race. It is okay to update the information twice, so
just use Overwrite instead of Put to update the SafeMap.
Bug: 275117853
Test: art/test.py -t 2246
Change-Id: I51fc976a85828311fb031eca65d21da201ce9ab6
diff --git a/runtime/trace.cc b/runtime/trace.cc
index 051a600..e23ff05 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -621,6 +621,23 @@
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);
+ // In tests, we destroy VM after already detaching the current thread. When a thread is
+ // detached we record the information about the threads_list_. We re-attach the current
+ // thread again as a "Shutdown thread" in the process of shutting down. So don't record
+ // information about shutdown threads.
+ if (name.compare("Shutdown thread") != 0) {
+ // This information is updated here when stopping tracing and also when a thread is
+ // detaching. In thread detach, we first update this information and then remove the
+ // thread from the list of active threads. If the tracing was stopped in between these
+ // events, we can see two updates for the same thread. Since we need a trace_lock_ it
+ // isn't easy to prevent this race (for ex: update this information when holding
+ // thread_list_lock_). It is harmless to do two updates so just use overwrite here.
+ the_trace->threads_list_.Overwrite(thread->GetTid(), name);
+ }
}
}
@@ -1242,28 +1259,17 @@
}
}
-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) {
- Thread* self = Thread::Current();
- for (const auto& it : exited_threads_) {
+ 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));
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) {
@@ -1271,9 +1277,7 @@
if (the_trace_ != nullptr) {
std::string name;
thread->GetThreadName(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);
+ the_trace_->threads_list_.Put(thread->GetTid(), name);
}
}
diff --git a/runtime/trace.h b/runtime/trace.h
index c3f903c..5f6f3c2 100644
--- a/runtime/trace.h
+++ b/runtime/trace.h
@@ -395,8 +395,9 @@
// Did we overflow the buffer recording traces?
bool overflow_;
- // Map of thread ids and names that have already exited.
- SafeMap<pid_t, std::string> exited_threads_;
+ // 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_;
// 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 cb4c0ca..90706b5 100644
--- a/test/2246-trace-stream/expected-stdout.txt
+++ b/test/2246-trace-stream/expected-stdout.txt
@@ -72,3 +72,85 @@
....<<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 942061f..daada7d 100644
--- a/test/2246-trace-stream/src/Main.java
+++ b/test/2246-trace-stream/src/Main.java
@@ -36,10 +36,9 @@
testTracing(
/* streaming=*/true, stream_parser, BaseTraceParser.STREAMING_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);
+ 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 3219591..db0450f 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");
+ String[] threadInfo = line.split("\t", 2);
threadIdMap.put(Integer.decode(threadInfo[0]), threadInfo[1]);
line = readLine();
}