From cebc409da6aa3e54756f6938e249d0d086b6bf01 Mon Sep 17 00:00:00 2001 From: John Knox Date: Mon, 9 Jul 2018 07:45:59 -0700 Subject: [PATCH] Change SonarInitConfig to take two EventBases Summary: We currently give sonar one event base from java / obj-c code to use for scheduling tasks. This causes a problem, because we schedule reconnect tasks on the event base, and then these tasks interact with rsocket which schedules it's own tasks. The problem is that we're passing rsocket the same event base. So the reconnect code executes and blocks waiting for rsocket to connect. But rsocket can never connect because it never executes because that thread is blocked, so we get deadlock. This is the first step which just changes the interface to pass two event bases. The consumers will be changed to pass in different threads next. Reviewed By: danielbuechele Differential Revision: D8748350 fbshipit-source-id: 481d6f23644f28fd0f1c786252605287019c999c --- android/android/AndroidSonarClient.java | 1 + android/android/SonarClientImpl.java | 3 ++- android/android/sonar.cpp | 6 ++++-- iOS/SonarKit/SonarClient.mm | 5 +++-- xplat/Sonar/SonarInitConfig.h | 7 ++++++- xplat/Sonar/SonarWebSocketImpl.cpp | 14 +++++++------- xplat/Sonar/SonarWebSocketImpl.h | 3 ++- 7 files changed, 25 insertions(+), 14 deletions(-) diff --git a/android/android/AndroidSonarClient.java b/android/android/AndroidSonarClient.java index 963799c9d..40c9a7ed4 100644 --- a/android/android/AndroidSonarClient.java +++ b/android/android/AndroidSonarClient.java @@ -24,6 +24,7 @@ public final class AndroidSonarClient { final Context app = context.getApplicationContext(); SonarClientImpl.init( + sSonarThread.getEventBase(), sSonarThread.getEventBase(), getServerHost(app), "Android", diff --git a/android/android/SonarClientImpl.java b/android/android/SonarClientImpl.java index eae290a4d..72c250d62 100644 --- a/android/android/SonarClientImpl.java +++ b/android/android/SonarClientImpl.java @@ -29,7 +29,8 @@ class SonarClientImpl implements SonarClient { } public static native void init( - EventBase eventBase, + EventBase callbackWorker, + EventBase connectionWorker, String host, String os, String device, diff --git a/android/android/sonar.cpp b/android/android/sonar.cpp index 874c91774..22d7f403c 100644 --- a/android/android/sonar.cpp +++ b/android/android/sonar.cpp @@ -268,7 +268,8 @@ class JSonarClient : public jni::HybridClass { static void init( jni::alias_ref, - JEventBase* jEventBase, + JEventBase* callbackWorker, + JEventBase* connectionWorker, const std::string host, const std::string os, const std::string device, @@ -287,7 +288,8 @@ class JSonarClient : public jni::HybridClass { std::move(appId), std::move(privateAppDirectory) }, - jEventBase->eventBase(), + callbackWorker->eventBase(), + connectionWorker->eventBase() }); } diff --git a/iOS/SonarKit/SonarClient.mm b/iOS/SonarKit/SonarClient.mm index c4915bfc3..b534c8c10 100644 --- a/iOS/SonarKit/SonarClient.mm +++ b/iOS/SonarKit/SonarClient.mm @@ -22,7 +22,7 @@ using WrapperPlugin = facebook::sonar::SonarCppWrapperPlugin; @implementation SonarClient { facebook::sonar::SonarClient *_cppClient; - folly::ScopedEventBaseThread eventBaseThread; + folly::ScopedEventBaseThread sonarThread; #if !TARGET_OS_SIMULATOR // SKPortForwardingServer *_server; #endif @@ -69,7 +69,8 @@ using WrapperPlugin = facebook::sonar::SonarCppWrapperPlugin; [appId UTF8String], [privateAppDirectory UTF8String], }, - eventBaseThread.getEventBase(), + sonarThread.getEventBase(), + sonarThread.getEventBase() }); _cppClient = facebook::sonar::SonarClient::instance(); } diff --git a/xplat/Sonar/SonarInitConfig.h b/xplat/Sonar/SonarInitConfig.h index ae1fa87ea..c594bd44b 100644 --- a/xplat/Sonar/SonarInitConfig.h +++ b/xplat/Sonar/SonarInitConfig.h @@ -33,7 +33,12 @@ struct SonarInitConfig { /** EventBase on which client callbacks should be called. */ - folly::EventBase* worker; + folly::EventBase* callbackWorker; + + /** + EventBase to be used to maintain the network connection. + */ + folly::EventBase* connectionWorker; }; } // namespace sonar diff --git a/xplat/Sonar/SonarWebSocketImpl.cpp b/xplat/Sonar/SonarWebSocketImpl.cpp index 343aa7956..9f06fa221 100644 --- a/xplat/Sonar/SonarWebSocketImpl.cpp +++ b/xplat/Sonar/SonarWebSocketImpl.cpp @@ -90,7 +90,7 @@ class Responder : public rsocket::RSocketResponder { }; SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config) - : deviceData_(config.deviceData), worker_(config.worker) {} + : deviceData_(config.deviceData), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker) {} SonarWebSocketImpl::~SonarWebSocketImpl() { stop(); @@ -98,7 +98,7 @@ SonarWebSocketImpl::~SonarWebSocketImpl() { void SonarWebSocketImpl::start() { folly::makeFuture() - .via(worker_->getEventBase()) + .via(sonarEventBase_->getEventBase()) .delayedUnsafe(std::chrono::milliseconds(0)) .then([this]() { startSync(); }); } @@ -135,7 +135,7 @@ void SonarWebSocketImpl::doCertificateExchange() { client_ = rsocket::RSocket::createConnectedClient( std::make_unique( - *worker_->getEventBase(), std::move(address)), + *connectionEventBase_->getEventBase(), std::move(address)), std::move(parameters), nullptr, std::chrono::seconds(connectionKeepaliveSeconds), // keepaliveInterval @@ -170,7 +170,7 @@ void SonarWebSocketImpl::connectSecurely() { client_ = rsocket::RSocket::createConnectedClient( std::make_unique( - *worker_->getEventBase(), + *connectionEventBase_->getEventBase(), std::move(address), std::move(sslContext)), std::move(parameters), @@ -184,7 +184,7 @@ void SonarWebSocketImpl::connectSecurely() { void SonarWebSocketImpl::reconnect() { folly::makeFuture() - .via(worker_->getEventBase()) + .via(sonarEventBase_->getEventBase()) .delayedUnsafe(std::chrono::seconds(reconnectIntervalSeconds)) .then([this]() { startSync(); }); } @@ -203,7 +203,7 @@ void SonarWebSocketImpl::setCallbacks(Callbacks* callbacks) { } void SonarWebSocketImpl::sendMessage(const folly::dynamic& message) { - worker_->add([this, message]() { + sonarEventBase_->add([this, message]() { if (client_) { client_->getRequester() ->fireAndForget(rsocket::Payload(folly::toJson(message))) @@ -239,7 +239,7 @@ void SonarWebSocketImpl::requestSignedCertFromSonar() { folly::dynamic message = folly::dynamic::object("method", "signCertificate")( "csr", csr.c_str())("destination", absoluteFilePath("").c_str()); - worker_->add([this, message]() { + sonarEventBase_->add([this, message]() { client_->getRequester() ->fireAndForget(rsocket::Payload(folly::toJson(message))) ->subscribe([this]() { diff --git a/xplat/Sonar/SonarWebSocketImpl.h b/xplat/Sonar/SonarWebSocketImpl.h index 2e74ef0f6..7d91fcd18 100644 --- a/xplat/Sonar/SonarWebSocketImpl.h +++ b/xplat/Sonar/SonarWebSocketImpl.h @@ -47,7 +47,8 @@ class SonarWebSocketImpl : public SonarWebSocket { Callbacks* callbacks_; DeviceData deviceData_; - folly::EventBase* worker_; + folly::EventBase* sonarEventBase_; + folly::EventBase* connectionEventBase_; std::unique_ptr client_; bool connectionIsTrusted_; int failedConnectionAttempts_ = 0;