Fix a race when dumping thread list for tracing
We may miss recording information about thread if a thread exits after
resetting the the_trace_ but before dumping the thread information. We
record the information about exiting threads when the_trace_ is active
but not after resetting the_trace_. So record the list of threads before
resetting the_trace_.
Use Put instead of Overwrite to record thread id -> thread name mapping.
Earlier due to the race we allowed overwrite which is no longer needed.
Also enable the non-stream trace which was failing because of the above
mentioned race.
Bug: 275117853
Test: art/test.py -t 2246
Change-Id: Ic8cab3297b9f9b5815dc080965d0326b42192f05
diff --git a/runtime/trace.cc b/runtime/trace.cc
index 051a600..bad4992 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -621,6 +621,11 @@
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);
}
}
@@ -1242,28 +1247,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 +1265,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();
}