Skip to content

Commit

Permalink
Fixing tsan races (#1307)
Browse files Browse the repository at this point in the history
  • Loading branch information
alyssawilk authored and mattklein123 committed Jul 22, 2017
1 parent fb22183 commit f247380
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
2 changes: 2 additions & 0 deletions test/integration/fake_upstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ FakeHttpConnectionPtr FakeUpstream::waitForHttpConnection(Event::Dispatcher& cli
ASSERT(!new_connections_.empty());
FakeHttpConnectionPtr connection(
new FakeHttpConnection(std::move(new_connections_.front()), stats_store_, http_type_));
connection->initialize();
new_connections_.pop_front();
connection->readDisable(false);
return connection;
Expand All @@ -295,6 +296,7 @@ FakeRawConnectionPtr FakeUpstream::waitForRawConnection() {

ASSERT(!new_connections_.empty());
FakeRawConnectionPtr connection(new FakeRawConnection(std::move(new_connections_.front())));
connection->initialize();
new_connections_.pop_front();
connection->readDisable(false);
return connection;
Expand Down
20 changes: 15 additions & 5 deletions test/integration/fake_upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,15 @@ class QueuedConnectionWrapper : public Network::ConnectionCallbacks {
: connection_(connection), parented_(false) {
connection_.addConnectionCallbacks(*this);
}
void set_parented() { parented_ = true; }
void set_parented() {
std::unique_lock<std::mutex> lock(lock_);
parented_ = true;
}
Network::Connection& connection() const { return connection_; }

// Network::ConnectionCallbacks
void onEvent(uint32_t events) override {
std::unique_lock<std::mutex> lock(lock_);
RELEASE_ASSERT(parented_ || (!(events & Network::ConnectionEvent::RemoteClose) &&
!(events & Network::ConnectionEvent::LocalClose)));
}
Expand All @@ -99,6 +103,7 @@ class QueuedConnectionWrapper : public Network::ConnectionCallbacks {
private:
Network::Connection& connection_;
bool parented_;
std::mutex lock_;
};

typedef std::unique_ptr<QueuedConnectionWrapper> QueuedConnectionWrapperPtr;
Expand All @@ -108,6 +113,7 @@ typedef std::unique_ptr<QueuedConnectionWrapper> QueuedConnectionWrapperPtr;
*/
class FakeConnectionBase : public Network::ConnectionCallbacks {
public:
~FakeConnectionBase() { ASSERT(initialized_); }
void close();
void readDisable(bool disable);
// By default waitForDisconnect assumes the next event is a disconnect and
Expand All @@ -120,18 +126,22 @@ class FakeConnectionBase : public Network::ConnectionCallbacks {
void onAboveWriteBufferHighWatermark() override {}
void onBelowWriteBufferLowWatermark() override {}

protected:
FakeConnectionBase(QueuedConnectionWrapperPtr connection_wrapper)
: connection_(connection_wrapper->connection()),
connection_wrapper_(std::move(connection_wrapper)) {
void initialize() {
initialized_ = true;
connection_wrapper_->set_parented();
connection_.dispatcher().post([this]() -> void { connection_.addConnectionCallbacks(*this); });
}

protected:
FakeConnectionBase(QueuedConnectionWrapperPtr connection_wrapper)
: connection_(connection_wrapper->connection()),
connection_wrapper_(std::move(connection_wrapper)) {}

Network::Connection& connection_;
std::mutex lock_;
std::condition_variable connection_event_;
bool disconnected_{};
bool initialized_{false};

private:
// We hold on to this as connection callbacks live for the entire life of the
Expand Down

0 comments on commit f247380

Please sign in to comment.