Commit 4506ae37 authored by Robin Cheng's avatar Robin Cheng Committed by Facebook GitHub Bot

Fix TSAN issues for //folly/io/async/test:async_test

Summary:
These are all test setup issues:
 - EventBase needs to be destructed before objects that callbacks may refer to.
 - A socket should not be operated on by a thread that is different from the event base it is associated with (in this case, the socket was being closed on the RSA computation thread).

Reviewed By: yfeldblum

Differential Revision: D23043485

fbshipit-source-id: 733868317aef09e32e79169aedb84a988de3bb41
parent 73a35da0
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include <folly/io/async/AsyncPipe.h> #include <folly/io/async/AsyncPipe.h>
#include <folly/io/async/AsyncSSLSocket.h> #include <folly/io/async/AsyncSSLSocket.h>
#include <folly/io/async/EventBase.h> #include <folly/io/async/EventBase.h>
#include <folly/io/async/EventBaseThread.h>
#include <folly/io/async/ScopedEventBaseThread.h> #include <folly/io/async/ScopedEventBaseThread.h>
#include <folly/net/NetOps.h> #include <folly/net/NetOps.h>
#include <folly/net/NetworkSocket.h> #include <folly/net/NetworkSocket.h>
...@@ -1574,19 +1575,18 @@ static int customRsaPrivEnc( ...@@ -1574,19 +1575,18 @@ static int customRsaPrivEnc(
auto asyncPipeWriter = folly::AsyncPipeWriter::newWriter( auto asyncPipeWriter = folly::AsyncPipeWriter::newWriter(
asyncJobEvb, folly::NetworkSocket(hand)); asyncJobEvb, folly::NetworkSocket(hand));
if (socket) {
LOG(INFO) << "Got a socket passed in, closing it...";
socket->closeNow();
}
asyncJobEvb->runInEventBaseThread([retptr = retptr, asyncJobEvb->runInEventBaseThread([retptr = retptr,
flen = flen, flen = flen,
from = from, from = from,
to = to, to = to,
padding = padding, padding = padding,
actualRSA = actualRSA, actualRSA = actualRSA,
writer = std::move(asyncPipeWriter), writer = std::move(asyncPipeWriter)]() {
socket = socket]() {
LOG(INFO) << "Running job"; LOG(INFO) << "Running job";
if (socket) {
LOG(INFO) << "Got a socket passed in, closing it...";
socket->closeNow();
}
*retptr = RSA_meth_get_priv_enc(RSA_PKCS1_OpenSSL())( *retptr = RSA_meth_get_priv_enc(RSA_PKCS1_OpenSSL())(
flen, from, to, actualRSA, padding); flen, from, to, actualRSA, padding);
LOG(INFO) << "Finished job, writing to pipe"; LOG(INFO) << "Finished job, writing to pipe";
...@@ -1781,7 +1781,8 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestFailure) { ...@@ -1781,7 +1781,8 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestFailure) {
TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) { TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) {
ASYNC_init_thread(1, 1); ASYNC_init_thread(1, 1);
EventBase eventBase; EventBase eventBase;
ScopedEventBaseThread jobEvbThread; std::optional<EventBaseThread> jobEvbThread;
jobEvbThread.emplace();
auto clientCtx = std::make_shared<SSLContext>(); auto clientCtx = std::make_shared<SSLContext>();
auto serverCtx = std::make_shared<SSLContext>(); auto serverCtx = std::make_shared<SSLContext>();
serverCtx->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"); serverCtx->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
...@@ -1790,7 +1791,7 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) { ...@@ -1790,7 +1791,7 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) {
serverCtx->loadClientCAList(kTestCA); serverCtx->loadClientCAList(kTestCA);
auto rsaPointers = auto rsaPointers =
setupCustomRSA(kTestCert, kTestKey, jobEvbThread.getEventBase()); setupCustomRSA(kTestCert, kTestKey, jobEvbThread->getEventBase());
CHECK(rsaPointers->dummyrsa); CHECK(rsaPointers->dummyrsa);
// up-refs dummyrsa // up-refs dummyrsa
SSL_CTX_use_RSAPrivateKey(serverCtx->getSSLCtx(), rsaPointers->dummyrsa); SSL_CTX_use_RSAPrivateKey(serverCtx->getSSLCtx(), rsaPointers->dummyrsa);
...@@ -1817,6 +1818,7 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) { ...@@ -1817,6 +1818,7 @@ TEST(AsyncSSLSocketTest, OpenSSL110AsyncTestClosedWithCallbackPending) {
EXPECT_TRUE(server.handshakeError_); EXPECT_TRUE(server.handshakeError_);
EXPECT_TRUE(client.handshakeError_); EXPECT_TRUE(client.handshakeError_);
ASYNC_cleanup_thread(); ASYNC_cleanup_thread();
jobEvbThread.reset();
} }
#endif // FOLLY_SANITIZE_ADDRESS #endif // FOLLY_SANITIZE_ADDRESS
......
...@@ -2293,8 +2293,8 @@ TEST(AsyncSocketTest, NumPendingMessagesInQueue) { ...@@ -2293,8 +2293,8 @@ TEST(AsyncSocketTest, NumPendingMessagesInQueue) {
serverSocket->getAddress(&serverAddress); serverSocket->getAddress(&serverAddress);
// Add a callback to accept connections // Add a callback to accept connections
folly::ScopedEventBaseThread cobThread("ioworker_test");
TestAcceptCallback acceptCallback; TestAcceptCallback acceptCallback;
folly::ScopedEventBaseThread cobThread("ioworker_test");
acceptCallback.setConnectionAcceptedFn( acceptCallback.setConnectionAcceptedFn(
[&](NetworkSocket /* fd */, const folly::SocketAddress& /* addr */) { [&](NetworkSocket /* fd */, const folly::SocketAddress& /* addr */) {
count++; count++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment