Title: [250329] trunk/Source/WebKit
- Revision
- 250329
- Author
- cdu...@apple.com
- Date
- 2019-09-24 18:56:50 -0700 (Tue, 24 Sep 2019)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (250328 => 250329)
--- trunk/Source/WebKit/ChangeLog 2019-09-25 01:46:47 UTC (rev 250328)
+++ trunk/Source/WebKit/ChangeLog 2019-09-25 01:56:50 UTC (rev 250329)
@@ -1,3 +1,30 @@
+2019-09-24 Chris Dumez <cdu...@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.r...@sony.com>
[WinCairo] Start RemoteInspectorServer
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (250328 => 250329)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-25 01:46:47 UTC (rev 250328)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-25 01:56:50 UTC (rev 250329)
@@ -111,34 +111,34 @@
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;
+ case AuxiliaryProcessProxy::State::Terminated:
+ return true;
+ case AuxiliaryProcessProxy::State::Running:
+ break;
+ }
-#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))
+ auto pid = processIdentifier();
+ if (!pid)
return true;
- if (errno == ESRCH) {
- // No process can be found corresponding to that specified by pid.
- return false;
- }
-
- RELEASE_LOG_ERROR(Process, "kill() returned unexpected error %d", errno);
- 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
}
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h (250328 => 250329)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-09-25 01:46:47 UTC (rev 250328)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h 2019-09-25 01:56:50 UTC (rev 250329)
@@ -99,6 +99,7 @@
};
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 @@
private:
virtual void connectionWillOpen(IPC::Connection&);
virtual void processWillShutDown(IPC::Connection&) = 0;
- static bool isRunningProcessPID(ProcessID);
struct PendingMessage {
std::unique_ptr<IPC::Encoder> encoder;
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (250328 => 250329)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-09-25 01:46:47 UTC (rev 250328)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-09-25 01:56:50 UTC (rev 250329)
@@ -807,7 +807,7 @@
// 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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
