[iOS] Regression(r249703) frequent 'kill() returned unexpected error'…

… log messages

https://bugs.webkit.org/show_bug.cgi?id=202173

Reviewed by Geoffrey Garen.

The kill(pid, 0) command actually fails with an EPERM error when there is a process
running with the given pid, and this is causing us to log a lot of errors. The good
news is that we merely want to know that there is no process with the given PID and
we correctly get a ESRCH error in this case. I renamed the function from
isRunningProcessPID() to wasTerminated() and only check for ESRCH error now. I no
longer log any error otherwise since this is expected.

Also, for performance reason, I no longer call kill(pid, 0) from inside
AuxiliaryProcessProxy::state() as it gets called a lot. I instead only call it from
AuxiliaryProcessProxy::wasTerminated() and call it from
WebProcessPool::tryTakePrewarmedProcess().

* UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::state const):
(WebKit::AuxiliaryProcessProxy::wasTerminated const):
(WebKit::AuxiliaryProcessProxy::isRunningProcessPID): Deleted.
* UIProcess/AuxiliaryProcessProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::tryTakePrewarmedProcess):


Canonical link: https://commits.webkit.org/215772@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250329 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,30 @@
+2019-09-24  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] Regression(r249703) frequent 'kill() returned unexpected error' log messages
+        https://bugs.webkit.org/show_bug.cgi?id=202173
+
+        Reviewed by Geoffrey Garen.
+
+        The kill(pid, 0) command actually fails with an EPERM error when there is a process
+        running with the given pid, and this is causing us to log a lot of errors. The good
+        news is that we merely want to know that there is no process with the given PID and
+        we correctly get a ESRCH error in this case. I renamed the function from
+        isRunningProcessPID() to wasTerminated() and only check for ESRCH error now. I no
+        longer log any error otherwise since this is expected.
+
+        Also, for performance reason, I no longer call kill(pid, 0) from inside
+        AuxiliaryProcessProxy::state() as it gets called a lot. I instead only call it from
+        AuxiliaryProcessProxy::wasTerminated() and call it from
+        WebProcessPool::tryTakePrewarmedProcess().
+
+        * UIProcess/AuxiliaryProcessProxy.cpp:
+        (WebKit::AuxiliaryProcessProxy::state const):
+        (WebKit::AuxiliaryProcessProxy::wasTerminated const):
+        (WebKit::AuxiliaryProcessProxy::isRunningProcessPID): Deleted.
+        * UIProcess/AuxiliaryProcessProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::tryTakePrewarmedProcess):
+
 2019-09-24  Christopher Reid  <chris.reid@sony.com>
 
         [WinCairo] Start RemoteInspectorServer

Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp

@@ -111,34 +111,34 @@ AuxiliaryProcessProxy::State AuxiliaryProcessProxy::state() const
     if (m_processLauncher && m_processLauncher->isLaunching())
         return AuxiliaryProcessProxy::State::Launching;
 
-    // There is sometimes a delay until we get the notification from mach about the connection getting closed.
-    // To help detect terminated process earlier, we also check that the PID is for a valid running process.
-    if (!m_connection || !isRunningProcessPID(processIdentifier()))
+    if (!m_connection)
         return AuxiliaryProcessProxy::State::Terminated;
 
     return AuxiliaryProcessProxy::State::Running;
 }
 
-bool AuxiliaryProcessProxy::isRunningProcessPID(ProcessID pid)
+bool AuxiliaryProcessProxy::wasTerminated() const
 {
-    if (!pid)
+    switch (state()) {
+    case AuxiliaryProcessProxy::State::Launching:
         return false;
-
-#if PLATFORM(COCOA)
-    // Use kill() with a signal of 0 to check if there is actually still a process with the given PID.
-    if (!kill(pid, 0))
+    case AuxiliaryProcessProxy::State::Terminated:
         return true;
-
-    if (errno == ESRCH) {
-        // No process can be found corresponding to that specified by pid.
-        return false;
+    case AuxiliaryProcessProxy::State::Running:
+        break;
     }
 
-    RELEASE_LOG_ERROR(Process, "kill() returned unexpected error %d", errno);
-    return true;
+    auto pid = processIdentifier();
+    if (!pid)
+        return true;
+
+#if PLATFORM(COCOA)
+    // Use kill() with a signal of 0 to make sure there is indeed still a process with the given PID.
+    // This is needed because it sometimes takes a little bit of time for us to get notified that a process
+    // was terminated.
+    return kill(pid, 0) && errno == ESRCH;
 #else
-    UNUSED_PARAM(pid);
-    return true;
+    return false;
 #endif
 }
 

Source/WebKit/UIProcess/AuxiliaryProcessProxy.h

@@ -99,6 +99,7 @@ class AuxiliaryProcessProxy : ProcessLauncher::Client, public IPC::Connection::C
     };
     State state() const;
     bool isLaunching() const { return state() == State::Launching; }
+    bool wasTerminated() const;
 
     ProcessID processIdentifier() const { return m_processLauncher ? m_processLauncher->processIdentifier() : 0; }
 
@@ -124,7 +125,6 @@ class AuxiliaryProcessProxy : ProcessLauncher::Client, public IPC::Connection::C
 private:
     virtual void connectionWillOpen(IPC::Connection&);
     virtual void processWillShutDown(IPC::Connection&) = 0;
-    static bool isRunningProcessPID(ProcessID);
 
     struct PendingMessage {
         std::unique_ptr<IPC::Encoder> encoder;

Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -807,7 +807,7 @@ RefPtr<WebProcessProxy> WebProcessPool::tryTakePrewarmedProcess(WebsiteDataStore
 
     // There is sometimes a delay until we get notified that a prewarmed process has been terminated (e.g. after resuming
     // from suspension) so make sure the process is still running here before deciding to use it.
-    if (m_prewarmedProcess->state() == AuxiliaryProcessProxy::State::Terminated) {
+    if (m_prewarmedProcess->wasTerminated()) {
         RELEASE_LOG_ERROR(Process, "Not using prewarmed process %d because it has been terminated", m_prewarmedProcess->processIdentifier());
         m_prewarmedProcess = nullptr;
         return nullptr;