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
This commit is contained in:
John Knox
2018-07-09 07:45:59 -07:00
committed by Facebook Github Bot
parent 7ed154c510
commit cebc409da6
7 changed files with 25 additions and 14 deletions

View File

@@ -24,6 +24,7 @@ public final class AndroidSonarClient {
final Context app = context.getApplicationContext(); final Context app = context.getApplicationContext();
SonarClientImpl.init( SonarClientImpl.init(
sSonarThread.getEventBase(),
sSonarThread.getEventBase(), sSonarThread.getEventBase(),
getServerHost(app), getServerHost(app),
"Android", "Android",

View File

@@ -29,7 +29,8 @@ class SonarClientImpl implements SonarClient {
} }
public static native void init( public static native void init(
EventBase eventBase, EventBase callbackWorker,
EventBase connectionWorker,
String host, String host,
String os, String os,
String device, String device,

View File

@@ -268,7 +268,8 @@ class JSonarClient : public jni::HybridClass<JSonarClient> {
static void init( static void init(
jni::alias_ref<jclass>, jni::alias_ref<jclass>,
JEventBase* jEventBase, JEventBase* callbackWorker,
JEventBase* connectionWorker,
const std::string host, const std::string host,
const std::string os, const std::string os,
const std::string device, const std::string device,
@@ -287,7 +288,8 @@ class JSonarClient : public jni::HybridClass<JSonarClient> {
std::move(appId), std::move(appId),
std::move(privateAppDirectory) std::move(privateAppDirectory)
}, },
jEventBase->eventBase(), callbackWorker->eventBase(),
connectionWorker->eventBase()
}); });
} }

View File

@@ -22,7 +22,7 @@ using WrapperPlugin = facebook::sonar::SonarCppWrapperPlugin;
@implementation SonarClient { @implementation SonarClient {
facebook::sonar::SonarClient *_cppClient; facebook::sonar::SonarClient *_cppClient;
folly::ScopedEventBaseThread eventBaseThread; folly::ScopedEventBaseThread sonarThread;
#if !TARGET_OS_SIMULATOR #if !TARGET_OS_SIMULATOR
// SKPortForwardingServer *_server; // SKPortForwardingServer *_server;
#endif #endif
@@ -69,7 +69,8 @@ using WrapperPlugin = facebook::sonar::SonarCppWrapperPlugin;
[appId UTF8String], [appId UTF8String],
[privateAppDirectory UTF8String], [privateAppDirectory UTF8String],
}, },
eventBaseThread.getEventBase(), sonarThread.getEventBase(),
sonarThread.getEventBase()
}); });
_cppClient = facebook::sonar::SonarClient::instance(); _cppClient = facebook::sonar::SonarClient::instance();
} }

View File

@@ -33,7 +33,12 @@ struct SonarInitConfig {
/** /**
EventBase on which client callbacks should be called. 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 } // namespace sonar

View File

@@ -90,7 +90,7 @@ class Responder : public rsocket::RSocketResponder {
}; };
SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config) SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config)
: deviceData_(config.deviceData), worker_(config.worker) {} : deviceData_(config.deviceData), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker) {}
SonarWebSocketImpl::~SonarWebSocketImpl() { SonarWebSocketImpl::~SonarWebSocketImpl() {
stop(); stop();
@@ -98,7 +98,7 @@ SonarWebSocketImpl::~SonarWebSocketImpl() {
void SonarWebSocketImpl::start() { void SonarWebSocketImpl::start() {
folly::makeFuture() folly::makeFuture()
.via(worker_->getEventBase()) .via(sonarEventBase_->getEventBase())
.delayedUnsafe(std::chrono::milliseconds(0)) .delayedUnsafe(std::chrono::milliseconds(0))
.then([this]() { startSync(); }); .then([this]() { startSync(); });
} }
@@ -135,7 +135,7 @@ void SonarWebSocketImpl::doCertificateExchange() {
client_ = client_ =
rsocket::RSocket::createConnectedClient( rsocket::RSocket::createConnectedClient(
std::make_unique<rsocket::TcpConnectionFactory>( std::make_unique<rsocket::TcpConnectionFactory>(
*worker_->getEventBase(), std::move(address)), *connectionEventBase_->getEventBase(), std::move(address)),
std::move(parameters), std::move(parameters),
nullptr, nullptr,
std::chrono::seconds(connectionKeepaliveSeconds), // keepaliveInterval std::chrono::seconds(connectionKeepaliveSeconds), // keepaliveInterval
@@ -170,7 +170,7 @@ void SonarWebSocketImpl::connectSecurely() {
client_ = client_ =
rsocket::RSocket::createConnectedClient( rsocket::RSocket::createConnectedClient(
std::make_unique<rsocket::TcpConnectionFactory>( std::make_unique<rsocket::TcpConnectionFactory>(
*worker_->getEventBase(), *connectionEventBase_->getEventBase(),
std::move(address), std::move(address),
std::move(sslContext)), std::move(sslContext)),
std::move(parameters), std::move(parameters),
@@ -184,7 +184,7 @@ void SonarWebSocketImpl::connectSecurely() {
void SonarWebSocketImpl::reconnect() { void SonarWebSocketImpl::reconnect() {
folly::makeFuture() folly::makeFuture()
.via(worker_->getEventBase()) .via(sonarEventBase_->getEventBase())
.delayedUnsafe(std::chrono::seconds(reconnectIntervalSeconds)) .delayedUnsafe(std::chrono::seconds(reconnectIntervalSeconds))
.then([this]() { startSync(); }); .then([this]() { startSync(); });
} }
@@ -203,7 +203,7 @@ void SonarWebSocketImpl::setCallbacks(Callbacks* callbacks) {
} }
void SonarWebSocketImpl::sendMessage(const folly::dynamic& message) { void SonarWebSocketImpl::sendMessage(const folly::dynamic& message) {
worker_->add([this, message]() { sonarEventBase_->add([this, message]() {
if (client_) { if (client_) {
client_->getRequester() client_->getRequester()
->fireAndForget(rsocket::Payload(folly::toJson(message))) ->fireAndForget(rsocket::Payload(folly::toJson(message)))
@@ -239,7 +239,7 @@ void SonarWebSocketImpl::requestSignedCertFromSonar() {
folly::dynamic message = folly::dynamic::object("method", "signCertificate")( folly::dynamic message = folly::dynamic::object("method", "signCertificate")(
"csr", csr.c_str())("destination", absoluteFilePath("").c_str()); "csr", csr.c_str())("destination", absoluteFilePath("").c_str());
worker_->add([this, message]() { sonarEventBase_->add([this, message]() {
client_->getRequester() client_->getRequester()
->fireAndForget(rsocket::Payload(folly::toJson(message))) ->fireAndForget(rsocket::Payload(folly::toJson(message)))
->subscribe([this]() { ->subscribe([this]() {

View File

@@ -47,7 +47,8 @@ class SonarWebSocketImpl : public SonarWebSocket {
Callbacks* callbacks_; Callbacks* callbacks_;
DeviceData deviceData_; DeviceData deviceData_;
folly::EventBase* worker_; folly::EventBase* sonarEventBase_;
folly::EventBase* connectionEventBase_;
std::unique_ptr<rsocket::RSocketClient> client_; std::unique_ptr<rsocket::RSocketClient> client_;
bool connectionIsTrusted_; bool connectionIsTrusted_;
int failedConnectionAttempts_ = 0; int failedConnectionAttempts_ = 0;