- Revision
- 219328
- Author
- [email protected]
- Date
- 2017-07-11 07:53:18 -0700 (Tue, 11 Jul 2017)
Log Message
NetworkProcess should close listening WebRTC sockets when being suspended
https://bugs.webkit.org/show_bug.cgi?id=174270
rdar://problem/33139844
Patch by Youenn Fablet <[email protected]> on 2017-07-11
Reviewed by Chris Dumez.
To prevent potential spinning of the NetworkProcess, NetworkProcess will now close listening sockets when being notified that it will be suspended.
When the network process is being suspended, it will stop creating listening sockets, until it resumes.
Future additional efforts might be to improve select/cancel so that we can stop listening sockets at resume time,
or to reimplement part of the stack using CFStream.
Tested through manual testing by going to a website doing WebRTC, homing out so that the network process is suspended and reopening Safari.
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::cleanupForSuspension):
Proxying call to clean for suspension to rtc provider so that it will
close listening sockets.
(WebKit::NetworkConnectionToWebProcess::resumeFromSuspension): Authorizing back listening sockets.
* NetworkProcess/NetworkConnectionToWebProcess.h:
(WebKit::NetworkConnectionToWebProcess::cleanupForSuspension):
Clean-up is done asynchronously as it can happen in background threads.
Hence why passing a callback as parameter.
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::notifyProcessReadyToSuspend): Helper routine
to notify UI process that network process clean up is done.
(WebKit::TaskCounter::TaskCounter): Helper class to call notifyProcessReadyToSuspend when sded.
(WebKit::TaskCounter::~TaskCounter):
(WebKit::NetworkProcess::actualPrepareToSuspend): Doing the clean-up for each connection that needs it.
Making sure to notify UI process of clean-up being completed once all connections are cleaned.
(WebKit::NetworkProcess::processWillSuspendImminently):
(WebKit::NetworkProcess::prepareToSuspend):
(WebKit::NetworkProcess::processDidResume): Reenable listening sockets.
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/webrtc/LibWebRTCSocketClient.cpp:
(WebKit::LibWebRTCSocketClient::LibWebRTCSocketClient):
* NetworkProcess/webrtc/LibWebRTCSocketClient.h: Adding type getter and making close public.
Used by NetworkRTCProvider to identifiy listening sockets and close them.
* NetworkProcess/webrtc/NetworkRTCProvider.cpp:
(WebKit::NetworkRTCProvider::closeListeningSockets): We close the webrtc socket
and we also notify the Web Process that the socket is closed so that it can take actions to recreate some if needed.
(WebKit::NetworkRTCProvider::finishClosingListeningSockets):
* NetworkProcess/webrtc/NetworkRTCProvider.h:
(WebKit::NetworkRTCProvider::authorizeListeningSockets): Authorize creation of listening sockets.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (219327 => 219328)
--- trunk/Source/WebKit2/ChangeLog 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/ChangeLog 2017-07-11 14:53:18 UTC (rev 219328)
@@ -1,3 +1,49 @@
+2017-07-11 Youenn Fablet <[email protected]>
+
+ NetworkProcess should close listening WebRTC sockets when being suspended
+ https://bugs.webkit.org/show_bug.cgi?id=174270
+ rdar://problem/33139844
+
+ Reviewed by Chris Dumez.
+
+ To prevent potential spinning of the NetworkProcess, NetworkProcess will now close listening sockets when being notified that it will be suspended.
+ When the network process is being suspended, it will stop creating listening sockets, until it resumes.
+ Future additional efforts might be to improve select/cancel so that we can stop listening sockets at resume time,
+ or to reimplement part of the stack using CFStream.
+
+ Tested through manual testing by going to a website doing WebRTC, homing out so that the network process is suspended and reopening Safari.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::cleanupForSuspension):
+ Proxying call to clean for suspension to rtc provider so that it will
+ close listening sockets.
+ (WebKit::NetworkConnectionToWebProcess::resumeFromSuspension): Authorizing back listening sockets.
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+ (WebKit::NetworkConnectionToWebProcess::cleanupForSuspension):
+ Clean-up is done asynchronously as it can happen in background threads.
+ Hence why passing a callback as parameter.
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::notifyProcessReadyToSuspend): Helper routine
+ to notify UI process that network process clean up is done.
+ (WebKit::TaskCounter::TaskCounter): Helper class to call notifyProcessReadyToSuspend when sded.
+ (WebKit::TaskCounter::~TaskCounter):
+ (WebKit::NetworkProcess::actualPrepareToSuspend): Doing the clean-up for each connection that needs it.
+ Making sure to notify UI process of clean-up being completed once all connections are cleaned.
+ (WebKit::NetworkProcess::processWillSuspendImminently):
+ (WebKit::NetworkProcess::prepareToSuspend):
+ (WebKit::NetworkProcess::processDidResume): Reenable listening sockets.
+ * NetworkProcess/NetworkProcess.h:
+ * NetworkProcess/webrtc/LibWebRTCSocketClient.cpp:
+ (WebKit::LibWebRTCSocketClient::LibWebRTCSocketClient):
+ * NetworkProcess/webrtc/LibWebRTCSocketClient.h: Adding type getter and making close public.
+ Used by NetworkRTCProvider to identifiy listening sockets and close them.
+ * NetworkProcess/webrtc/NetworkRTCProvider.cpp:
+ (WebKit::NetworkRTCProvider::closeListeningSockets): We close the webrtc socket
+ and we also notify the Web Process that the socket is closed so that it can take actions to recreate some if needed.
+ (WebKit::NetworkRTCProvider::finishClosingListeningSockets):
+ * NetworkProcess/webrtc/NetworkRTCProvider.h:
+ (WebKit::NetworkRTCProvider::authorizeListeningSockets): Authorize creation of listening sockets.
+
2017-07-10 Brent Fulgham <[email protected]>
Resource Load Statistics: Prune statistics in orders of importance
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2017-07-11 14:53:18 UTC (rev 219328)
@@ -189,6 +189,25 @@
m_networkSocketStreams.remove(identifier);
}
+void NetworkConnectionToWebProcess::cleanupForSuspension(Function<void()>&& completionHandler)
+{
+#if USE(LIBWEBRTC)
+ if (m_rtcProvider) {
+ m_rtcProvider->closeListeningSockets(WTFMove(completionHandler));
+ return;
+ }
+#endif
+ completionHandler();
+}
+
+void NetworkConnectionToWebProcess::endSuspension()
+{
+#if USE(LIBWEBRTC)
+ if (m_rtcProvider)
+ m_rtcProvider->authorizeListeningSockets();
+#endif
+}
+
void NetworkConnectionToWebProcess::scheduleResourceLoad(const NetworkResourceLoadParameters& loadParameters)
{
auto loader = NetworkResourceLoader::create(loadParameters, *this);
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2017-07-11 14:53:18 UTC (rev 219328)
@@ -64,6 +64,9 @@
RefPtr<WebCore::BlobDataFileReference> getBlobDataFileReferenceForPath(const String& path);
+ void cleanupForSuspension(Function<void()>&&);
+ void endSuspension();
+
private:
NetworkConnectionToWebProcess(IPC::Connection::Identifier);
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp 2017-07-11 14:53:18 UTC (rev 219328)
@@ -640,9 +640,36 @@
ChildProcess::terminate();
}
+// FIXME: We can remove this one by adapting RefCounter.
+class TaskCounter : public RefCounted<TaskCounter> {
+public:
+ explicit TaskCounter(Function<void()>&& callback) : m_callback(WTFMove(callback)) { }
+ ~TaskCounter() { m_callback(); };
+
+private:
+ Function<void()> m_callback;
+};
+
+void NetworkProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend shouldAcknowledgeWhenReadyToSuspend)
+{
+ lowMemoryHandler(Critical::Yes);
+
+ RefPtr<TaskCounter> delayedTaskCounter;
+ if (shouldAcknowledgeWhenReadyToSuspend == ShouldAcknowledgeWhenReadyToSuspend::Yes) {
+ delayedTaskCounter = adoptRef(new TaskCounter([this] {
+ RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::notifyProcessReadyToSuspend() Sending ProcessReadyToSuspend IPC message", this);
+ if (parentProcessConnection())
+ parentProcessConnection()->send(Messages::NetworkProcessProxy::ProcessReadyToSuspend(), 0);
+ }));
+ }
+
+ for (auto& connection : m_webProcessConnections)
+ connection->cleanupForSuspension([delayedTaskCounter] { });
+}
+
void NetworkProcess::processWillSuspendImminently(bool& handled)
{
- lowMemoryHandler(Critical::Yes);
+ actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
handled = true;
}
@@ -649,10 +676,7 @@
void NetworkProcess::prepareToSuspend()
{
RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::prepareToSuspend()", this);
- lowMemoryHandler(Critical::Yes);
-
- RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::prepareToSuspend() Sending ProcessReadyToSuspend IPC message", this);
- parentProcessConnection()->send(Messages::NetworkProcessProxy::ProcessReadyToSuspend(), 0);
+ actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::Yes);
}
void NetworkProcess::cancelPrepareToSuspend()
@@ -662,11 +686,15 @@
// message. And NetworkProcessProxy expects to receive either a NetworkProcessProxy::ProcessReadyToSuspend-
// or NetworkProcessProxy::DidCancelProcessSuspension- message, but not both.
RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::cancelPrepareToSuspend()", this);
+ for (auto& connection : m_webProcessConnections)
+ connection->endSuspension();
}
void NetworkProcess::processDidResume()
{
RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processDidResume()", this);
+ for (auto& connection : m_webProcessConnections)
+ connection->endSuspension();
}
void NetworkProcess::prefetchDNS(const String& hostname)
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkProcess.h (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/NetworkProcess.h 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkProcess.h 2017-07-11 14:53:18 UTC (rev 219328)
@@ -134,6 +134,9 @@
void lowMemoryHandler(Critical);
+ enum class ShouldAcknowledgeWhenReadyToSuspend { No, Yes };
+ void actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend);
+
// ChildProcess
void initializeProcess(const ChildProcessInitializationParameters&) override;
void initializeProcessName(const ChildProcessInitializationParameters&) override;
Modified: trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.cpp (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.cpp 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.cpp 2017-07-11 14:53:18 UTC (rev 219328)
@@ -39,6 +39,7 @@
LibWebRTCSocketClient::LibWebRTCSocketClient(uint64_t identifier, NetworkRTCProvider& rtcProvider, std::unique_ptr<rtc::AsyncPacketSocket>&& socket, Type type)
: m_identifier(identifier)
+ , m_type(type)
, m_rtcProvider(rtcProvider)
, m_socket(WTFMove(socket))
{
Modified: trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.h (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.h 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/webrtc/LibWebRTCSocketClient.h 2017-07-11 14:53:18 UTC (rev 219328)
@@ -54,11 +54,12 @@
LibWebRTCSocketClient(uint64_t identifier, NetworkRTCProvider&, std::unique_ptr<rtc::AsyncPacketSocket>&&, Type);
uint64_t identifier() const { return m_identifier; }
+ Type type() const { return m_type; }
+ void close();
private:
friend class NetworkRTCSocket;
- void close();
void setOption(int option, int value);
void sendTo(const WebCore::SharedBuffer&, const rtc::SocketAddress&, const rtc::PacketOptions&);
@@ -72,6 +73,7 @@
void signalAddressReady();
uint64_t m_identifier;
+ Type m_type;
NetworkRTCProvider& m_rtcProvider;
std::unique_ptr<rtc::AsyncPacketSocket> m_socket;
};
Modified: trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.cpp (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.cpp 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.cpp 2017-07-11 14:53:18 UTC (rev 219328)
@@ -109,6 +109,12 @@
void NetworkRTCProvider::createServerTCPSocket(uint64_t identifier, const RTCNetwork::SocketAddress& address, uint16_t minPort, uint16_t maxPort, int options)
{
+ if (!m_isListeningSocketAuthorized) {
+ if (m_connection)
+ m_connection->connection().send(Messages::WebRTCSocket::SignalClose(1), identifier);
+ return;
+ }
+
callOnRTCNetworkThread([this, identifier, address = RTCNetwork::isolatedCopy(address.value), minPort, maxPort, options]() {
std::unique_ptr<rtc::AsyncPacketSocket> socket(m_packetSocketFactory->CreateServerTcpSocket(address, minPort, maxPort, options));
createSocket(identifier, WTFMove(socket), LibWebRTCSocketClient::Type::ServerTCP);
@@ -215,6 +221,33 @@
resolver->rtcProvider.m_connection->connection().send(Messages::WebRTCResolver::SetResolvedAddress(addresses), resolver->identifier);
}
+void NetworkRTCProvider::closeListeningSockets(Function<void()>&& completionHandler)
+{
+ if (!m_isListeningSocketAuthorized) {
+ completionHandler();
+ return;
+ }
+
+ m_isListeningSocketAuthorized = false;
+ callOnRTCNetworkThread([this, completionHandler = WTFMove(completionHandler)]() mutable {
+ Vector<uint64_t> listeningSocketIdentifiers;
+ for (auto& keyValue : m_sockets) {
+ if (keyValue.value->type() == LibWebRTCSocketClient::Type::ServerTCP)
+ listeningSocketIdentifiers.append(keyValue.key);
+ }
+ for (auto id : listeningSocketIdentifiers)
+ m_sockets.get(id)->close();
+
+ callOnMainThread([provider = makeRef(*this), listeningSocketIdentifiers = WTFMove(listeningSocketIdentifiers), completionHandler = WTFMove(completionHandler)] {
+ if (provider->m_connection) {
+ for (auto identifier : listeningSocketIdentifiers)
+ provider->m_connection->connection().send(Messages::WebRTCSocket::SignalClose(ECONNABORTED), identifier);
+ }
+ completionHandler();
+ });
+ });
+}
+
struct NetworkMessageData : public rtc::MessageData {
NetworkMessageData(Ref<NetworkRTCProvider>&& rtcProvider, Function<void()>&& callback)
: rtcProvider(WTFMove(rtcProvider))
Modified: trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.h (219327 => 219328)
--- trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.h 2017-07-11 13:29:40 UTC (rev 219327)
+++ trunk/Source/WebKit2/NetworkProcess/webrtc/NetworkRTCProvider.h 2017-07-11 14:53:18 UTC (rev 219328)
@@ -48,7 +48,7 @@
class NetworkConnectionToWebProcess;
class NetworkRTCSocket;
-
+
class NetworkRTCProvider : public ThreadSafeRefCounted<NetworkRTCProvider>, public rtc::MessageHandler {
public:
static Ref<NetworkRTCProvider> create(NetworkConnectionToWebProcess& connection) { return adoptRef(*new NetworkRTCProvider(connection)); }
@@ -69,6 +69,9 @@
void newConnection(LibWebRTCSocketClient&, std::unique_ptr<rtc::AsyncPacketSocket>&&);
+ void closeListeningSockets(Function<void()>&&);
+ void authorizeListeningSockets() { m_isListeningSocketAuthorized = true; }
+
private:
explicit NetworkRTCProvider(NetworkConnectionToWebProcess&);
@@ -112,6 +115,7 @@
HashMap<uint64_t, std::unique_ptr<rtc::AsyncPacketSocket>> m_pendingIncomingSockets;
uint64_t m_incomingSocketIdentifier { 0 };
+ bool m_isListeningSocketAuthorized { true };
};
} // namespace WebKit
