Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h =================================================================== --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.h @@ -76,21 +76,12 @@ public: explicit ThreadLister(int pid); ~ThreadLister(); - // GetNextTID returns -1 if the list of threads is exhausted, or if there has - // been an error. - int GetNextTID(); - void Reset(); - bool error(); + bool ListThreads(InternalMmapVector *threads); private: - bool GetDirectoryEntries(); - int pid_; - int descriptor_; + int descriptor_ = -1; InternalMmapVector buffer_; - bool error_; - struct linux_dirent* entry_; - int bytes_read_; }; // Exposed for testing. Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc =================================================================== --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc @@ -905,47 +905,40 @@ #endif // !SANITIZER_SOLARIS // ThreadLister implementation. -ThreadLister::ThreadLister(int pid) - : pid_(pid), - descriptor_(-1), - buffer_(4096), - error_(true), - entry_((struct linux_dirent *)buffer_.data()), - bytes_read_(0) { +ThreadLister::ThreadLister(int pid) : pid_(pid), buffer_(4096) { + buffer_.resize(buffer_.capacity()); char task_directory_path[80]; internal_snprintf(task_directory_path, sizeof(task_directory_path), "/proc/%d/task/", pid); - uptr openrv = internal_open(task_directory_path, O_RDONLY | O_DIRECTORY); - if (internal_iserror(openrv)) { - error_ = true; + descriptor_ = internal_open(task_directory_path, O_RDONLY | O_DIRECTORY); + if (internal_iserror(descriptor_)) { Report("Can't open /proc/%d/task for reading.\n", pid); - } else { - error_ = false; - descriptor_ = openrv; } } -int ThreadLister::GetNextTID() { - int tid = -1; - do { - if (error_) - return -1; - if ((char *)entry_ >= &buffer_[bytes_read_] && !GetDirectoryEntries()) - return -1; - if (entry_->d_ino != 0 && entry_->d_name[0] >= '0' && - entry_->d_name[0] <= '9') { - // Found a valid tid. - tid = (int)internal_atoll(entry_->d_name); +bool ThreadLister::ListThreads(InternalMmapVector *threads) { + if (!descriptor_) return false; + internal_lseek(descriptor_, 0, SEEK_SET); + threads->clear(); + + while (uptr read = internal_getdents(descriptor_, + (struct linux_dirent *)buffer_.data(), + buffer_.size())) { + if (internal_iserror(read)) { + Report("Can't read directory entries from /proc/%d/task.\n", pid_); + return false; } - entry_ = (struct linux_dirent *)(((char *)entry_) + entry_->d_reclen); - } while (tid < 0); - return tid; -} -void ThreadLister::Reset() { - if (error_ || descriptor_ < 0) - return; - internal_lseek(descriptor_, 0, SEEK_SET); + for (uptr begin = (uptr)buffer_.data(), end = begin + read; begin < end;) { + struct linux_dirent *entry = (struct linux_dirent *)begin; + begin += entry->d_reclen; + if (entry->d_ino && *entry->d_name >= '0' && *entry->d_name <= '9') { + // Found a valid tid. + threads->push_back(internal_atoll(entry->d_name)); + } + } + } + return true; } ThreadLister::~ThreadLister() { @@ -953,24 +946,6 @@ internal_close(descriptor_); } -bool ThreadLister::error() { return error_; } - -bool ThreadLister::GetDirectoryEntries() { - CHECK_GE(descriptor_, 0); - CHECK_NE(error_, true); - bytes_read_ = internal_getdents( - descriptor_, (struct linux_dirent *)buffer_.data(), buffer_.size()); - if (internal_iserror(bytes_read_)) { - Report("Can't read directory entries from /proc/%d/task.\n", pid_); - error_ = true; - return false; - } else if (bytes_read_ == 0) { - return false; - } - entry_ = (struct linux_dirent *)buffer_.data(); - return true; -} - #if SANITIZER_WORDSIZE == 32 // Take care of unusable kernel area in top gigabyte. static uptr GetKernelAreaSize() { Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc =================================================================== --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc @@ -211,21 +211,23 @@ ThreadLister thread_lister(pid_); bool added_threads; bool first_iteration = true; + InternalMmapVector threads; + threads.reserve(128); do { // Run through the directory entries once. added_threads = false; - pid_t tid = thread_lister.GetNextTID(); - while (tid >= 0) { + if (!thread_lister.ListThreads(&threads)) { + ResumeAllThreads(); + return false; + } + for (int tid : threads) if (SuspendThread(tid)) added_threads = true; - tid = thread_lister.GetNextTID(); - } - if (thread_lister.error() || (first_iteration && !added_threads)) { + if (first_iteration && !added_threads) { // Detach threads and fail. ResumeAllThreads(); return false; } - thread_lister.Reset(); first_iteration = false; } while (added_threads); return true; Index: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc =================================================================== --- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc +++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc @@ -123,11 +123,9 @@ static std::vector ReadTidsToVector(ThreadLister *thread_lister) { std::vector listed_tids; - pid_t tid; - while ((tid = thread_lister->GetNextTID()) >= 0) - listed_tids.push_back(tid); - EXPECT_FALSE(thread_lister->error()); - return listed_tids; + InternalMmapVector threads(128); + EXPECT_TRUE(thread_lister->ListThreads(&threads)); + return std::vector(threads.begin(), threads.end()); } static bool Includes(std::vector first, std::vector second) { @@ -151,23 +149,20 @@ ASSERT_TRUE(Includes(listed_tids, tids_)); } -// Calling Reset() should not cause ThreadLister to forget any threads it's -// supposed to know about. -TEST_F(ThreadListerTest, ResetDoesNotForgetThreads) { +TEST_F(ThreadListerTest, DoNotForgetThreads) { ThreadLister thread_lister(getpid()); - // Run the loop body twice, because Reset() might behave differently if called - // on a freshly created object. + // Run the loop body twice, because ThreadLister might behave differently if + // called on a freshly created object. for (uptr i = 0; i < 2; i++) { - thread_lister.Reset(); std::vector listed_tids = ReadTidsToVector(&thread_lister); ASSERT_TRUE(Includes(listed_tids, tids_)); } } // If new threads have spawned during ThreadLister object's lifetime, calling -// Reset() should cause ThreadLister to recognize their existence. -TEST_F(ThreadListerTest, ResetMakesNewThreadsKnown) { +// relisting should cause ThreadLister to recognize their existence. +TEST_F(ThreadListerTest, NewThreads) { ThreadLister thread_lister(getpid()); std::vector threads_before_extra = ReadTidsToVector(&thread_lister); @@ -182,8 +177,6 @@ // so better check for that. ASSERT_FALSE(HasElement(threads_before_extra, extra_tid)); - thread_lister.Reset(); - std::vector threads_after_extra = ReadTidsToVector(&thread_lister); ASSERT_TRUE(HasElement(threads_after_extra, extra_tid)); }