Bug 1647695 - Make Windows GetProcInfo a bit more resilient;r=tarek
authorDavid Teller <dteller@mozilla.com>
Mon, 10 Aug 2020 10:15:44 +0000
changeset 544075 873dc7235673e6df48886f177cb3dda80108c6a2
parent 544074 2facd744d7c36989f9f0cf58021cefe525731f0c
child 544076 52b53f5387560ee1ac1e2873784518ba236fd658
push id37687
push userapavel@mozilla.com
push dateMon, 10 Aug 2020 21:36:34 +0000
treeherdermozilla-central@ee09cb88af17 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstarek
bugs1647695
milestone81.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1647695 - Make Windows GetProcInfo a bit more resilient;r=tarek GetProcInfo is designed to collect process & thread data while they're running, which means that it needs to handle the case in which a thread shuts down while we're looking at it. By design, in case of failure, GetProcInfo return partial data in case of failure. This patch ensures that we always either set the thread ID (which we use to display about:processes) correctly or remove the thread entirely from the list. Differential Revision: https://phabricator.services.mozilla.com/D86055
toolkit/components/aboutprocesses/tests/browser/browser_aboutprocesses.js
widget/windows/ProcInfo.cpp
--- a/toolkit/components/aboutprocesses/tests/browser/browser_aboutprocesses.js
+++ b/toolkit/components/aboutprocesses/tests/browser/browser_aboutprocesses.js
@@ -361,17 +361,17 @@ add_task(async function testAboutProcess
     let children = threadRow.children;
     let tidContent = children[0].textContent;
     let cpuUserContent = children[3].textContent;
     let cpuKernelContent = children[4].textContent;
     ++numberOfThreadsFound;
 
     info("Sanity checks: tid");
     let tid = Number.parseInt(tidContent);
-    Assert.ok(tid > 0);
+    Assert.notEqual(tid, 0, "The tid should be set");
     Assert.equal(tid, threadRow.thread.tid);
 
     info("Sanity checks: CPU (user)");
     testCpu(
       cpuUserContent,
       threadRow.thread.totalCpuUser,
       threadRow.thread.slopeCpuUser,
       HARDCODED_ASSUMPTIONS_THREAD
--- a/widget/windows/ProcInfo.cpp
+++ b/widget/windows/ProcInfo.cpp
@@ -131,40 +131,45 @@ RefPtr<ProcInfoPromise> GetProcInfo(nsTA
             return;
           }
 
           nsAutoHandle hThread(
               OpenThread(/* dwDesiredAccess = */ THREAD_QUERY_INFORMATION,
                          /* bInheritHandle = */ FALSE,
                          /* dwThreadId = */ te32.th32ThreadID));
           if (!hThread) {
-            // Cannot open thread. Not sure why, but let's attempt to find data
-            // on other threads.
+            // Cannot open thread. Not sure why, but let's erase this thread
+            // and attempt to find data on other threads.
+            processLookup->value().threads.RemoveLastElement();
             continue;
           }
 
+          threadInfo->tid = te32.th32ThreadID;
+
+          // Attempt to get thread times.
+          // If we fail, continue without this piece of information.
           FILETIME createTime, exitTime, kernelTime, userTime;
-          if (!GetThreadTimes(hThread.get(), &createTime, &exitTime,
-                              &kernelTime, &userTime)) {
-            continue;
+          if (GetThreadTimes(hThread.get(), &createTime, &exitTime, &kernelTime,
+                             &userTime)) {
+            threadInfo->cpuKernel = ToNanoSeconds(kernelTime);
+            threadInfo->cpuUser = ToNanoSeconds(userTime);
           }
 
+          // Attempt to get thread name.
+          // If we fail, continue without this piece of information.
           if (getThreadDescription) {
             PWSTR threadName = nullptr;
             if (getThreadDescription(hThread.get(), &threadName) &&
                 threadName) {
               threadInfo->name = threadName;
             }
             if (threadName) {
               LocalFree(threadName);
             }
           }
-          threadInfo->tid = te32.th32ThreadID;
-          threadInfo->cpuKernel = ToNanoSeconds(kernelTime);
-          threadInfo->cpuUser = ToNanoSeconds(userTime);
         }
 
         // ----- We're ready to return.
         holder->Resolve(std::move(gathered), __func__);
       });
 
   rv = target->Dispatch(r.forget(), NS_DISPATCH_NORMAL);
   if (NS_FAILED(rv)) {