Flipper | Fix potential for deadlock when stopping Flipper
Summary: There was an interesting problem in Flipper which I encountered. On closing our app we would deadlock. This was happening when joining the Flipper thread. Upon investigation I found that it happened only when Flipper was not connected to the app. And digging in I found that it's because we are continually trying to connect the `FlipperClient` to the Flipper app when there is no connection. This works great, but it continues even if you have asked Flipper to terminate. So this meant that the `EventBase` never gets chance to shutdown, as it keeps getting new work to do. The fix is to add a flag to `FlipperConnectionManagerImpl` to say if the conection is started or not. Then, if it's not started and we get a request to retry connection, then we will fail it. Reviewed By: jknoxville Differential Revision: D18780622 fbshipit-source-id: cce82cbb5c54e6d92ea16644c8a247bd2578ae26
This commit is contained in:
committed by
Facebook Github Bot
parent
15e1b25098
commit
d37c64c329
@@ -91,6 +91,12 @@ FlipperConnectionManagerImpl::~FlipperConnectionManagerImpl() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void FlipperConnectionManagerImpl::start() {
|
void FlipperConnectionManagerImpl::start() {
|
||||||
|
if (isStarted_) {
|
||||||
|
log("Already started");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
isStarted_ = true;
|
||||||
|
|
||||||
auto step = flipperState_->start("Start connection thread");
|
auto step = flipperState_->start("Start connection thread");
|
||||||
|
|
||||||
folly::makeFuture()
|
folly::makeFuture()
|
||||||
@@ -103,6 +109,10 @@ void FlipperConnectionManagerImpl::start() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void FlipperConnectionManagerImpl::startSync() {
|
void FlipperConnectionManagerImpl::startSync() {
|
||||||
|
if (!isStarted_) {
|
||||||
|
log("Not started");
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (!isRunningInOwnThread()) {
|
if (!isRunningInOwnThread()) {
|
||||||
log(WRONG_THREAD_EXIT_MSG);
|
log(WRONG_THREAD_EXIT_MSG);
|
||||||
return;
|
return;
|
||||||
@@ -219,6 +229,10 @@ void FlipperConnectionManagerImpl::connectSecurely() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void FlipperConnectionManagerImpl::reconnect() {
|
void FlipperConnectionManagerImpl::reconnect() {
|
||||||
|
if (!isStarted_) {
|
||||||
|
log("Not started");
|
||||||
|
return;
|
||||||
|
}
|
||||||
folly::makeFuture()
|
folly::makeFuture()
|
||||||
.via(flipperEventBase_->getEventBase())
|
.via(flipperEventBase_->getEventBase())
|
||||||
.delayed(std::chrono::seconds(reconnectIntervalSeconds))
|
.delayed(std::chrono::seconds(reconnectIntervalSeconds))
|
||||||
@@ -226,6 +240,12 @@ void FlipperConnectionManagerImpl::reconnect() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void FlipperConnectionManagerImpl::stop() {
|
void FlipperConnectionManagerImpl::stop() {
|
||||||
|
if (!isStarted_) {
|
||||||
|
log("Not started");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
isStarted_ = false;
|
||||||
|
|
||||||
if (client_) {
|
if (client_) {
|
||||||
client_->disconnect();
|
client_->disconnect();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -53,6 +53,7 @@ class FlipperConnectionManagerImpl : public FlipperConnectionManager {
|
|||||||
|
|
||||||
private:
|
private:
|
||||||
bool isOpen_ = false;
|
bool isOpen_ = false;
|
||||||
|
bool isStarted_ = false;
|
||||||
Callbacks* callbacks_;
|
Callbacks* callbacks_;
|
||||||
DeviceData deviceData_;
|
DeviceData deviceData_;
|
||||||
std::shared_ptr<FlipperState> flipperState_;
|
std::shared_ptr<FlipperState> flipperState_;
|
||||||
|
|||||||
Reference in New Issue
Block a user