Fix a bug in how ScopedServerInterfaceThread creates clients

Summary:
TLRD: Before this change trying to use ScopedServerInterfaceThread::newClient() to create client using binary protocol would crash the process.

ScopedServerInterfaceThread::newChannel takes executor, channel
functor, numThreads and protocol. The thing is ScopedServerInterfaceThread::newClient calls are passing protocol in place of numThreads.

Because protocol is just an enum (not enum class), it is implicitly converted into int and default protocol T_BINARY_PROTOCOL is used instead of T_COMPACT_PROTOCOL that is default for ScopedServerInterfaceThread::newClient.

This worked only because T_COMPACT_PROTOCOL is when implicitly converted to int results in value of 2 for numThreads. But, if one tries to actually create client with T_BINARY_PROTOCOL who's int value is 0, the process crashes with devision by 0.

Reviewed By: owwlo

Differential Revision: D75345838

fbshipit-source-id: 19253c61a22299e0a2ff44de6b49a11ec620d843

Author: Alex Zhavnerchik

third-party/thrift/src/thrift/lib/cpp2/test/util/ScopedServerInterfaceThreadTest.cpp

@@ -114,6 +114,16 @@ TEST(ScopedServerInterfaceThread, newClient) {
   EXPECT_EQ(6, cli->sync_add(-3, 9));
 }
 
+TEST(ScopedServerInterfaceThread, newClientBinary) {
+  ScopedServerInterfaceThread ssit(make_shared<SimpleServiceImpl>());
+
+  EventBase eb;
+  auto cli = ssit.newClient<SimpleServiceAsyncClient>(
+      &eb, RocketClientChannel::newChannel, protocol::T_BINARY_PROTOCOL);
+
+  EXPECT_EQ(6, cli->sync_add(-3, 9));
+}
+
 TEST(ScopedServerInterfaceThread, newClient_SemiFuture) {
   ScopedServerInterfaceThread ssit(make_shared<SimpleServiceImpl>());
 

third-party/thrift/src/thrift/lib/cpp2/transport/rocket/framing/parser/test/AlignedParserStrategyTest.cpp

@@ -496,7 +496,8 @@ TEST(AlignedParserStrategyTest, TestAlignmentWithRPC) {
   auto handler = std::make_shared<TestServiceHandler>();
   auto server =
       std::make_shared<ScopedServerInterfaceThread>(handler, [](auto&) {});
-  auto client = server->newClient<apache::thrift::Client<TestService>>();
+  auto client = server->newClient<apache::thrift::Client<TestService>>(
+      nullptr, RocketClientChannel::newChannel, protocol::T_BINARY_PROTOCOL);
 
   for (int i = 0; i < 20; ++i) {
     {

third-party/thrift/src/thrift/lib/cpp2/util/ScopedServerInterfaceThread-inl.h

@@ -232,8 +232,11 @@ std::unique_ptr<AsyncClientT> ScopedServerInterfaceThread::newClient(
     folly::Executor* callbackExecutor,
     ScopedServerInterfaceThread::MakeChannelFunc makeChannel,
     protocol::PROTOCOL_TYPES prot) const {
-  return std::make_unique<AsyncClientT>(
-      newChannel(callbackExecutor, std::move(makeChannel), prot));
+  return std::make_unique<AsyncClientT>(newChannel(
+      callbackExecutor,
+      std::move(makeChannel),
+      folly::hardware_concurrency(),
+      prot));
 }
 
 template <class AsyncClientT>