This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 0ecc2c7 messenger: adjust lock usage
0ecc2c7 is described below
commit 0ecc2c7715505fa6d5a03f8ef967a1a96d4f55d5
Author: Adar Dembo <[email protected]>
AuthorDate: Tue Sep 17 12:09:28 2019 -0700
messenger: adjust lock usage
The Messenger's lock is only intended to protect closing_, acceptor_pools_,
and rpc_services_. This change adjusts its usage to reflect that:
1. There's no need to take the lock in the destructor.
2. It was held for longer than necessary in QueueInboundCall.
3. It wasn't needed at all in DumpConnections.
The motivation for this was a TSAN lock inversion warning I saw in a
precommit job, between the Messenger lock and glog's vmodule lock. The
warning seems wrong (the vmodule lock is released after a VLOG statement
ends), but one way to avoid it altogether is to not take the Messenger lock
in its destructor.
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)
(pid=5867)
Cycle in lock order graph: M1870 (0x7b14000172f8) => M37857528269694952
(0x000000000000) => M1870
Mutex M37857528269694952 acquired here while holding mutex M1870 in main
thread:
#0 pthread_rwlock_wrlock
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1352
(kudu+0x4a360f)
#1 glog_internal_namespace_::Mutex::Lock()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/base/mutex.h:250:30
(libglog.so.0+0x1abe7)
#2
glog_internal_namespace_::MutexLock::MutexLock(glog_internal_namespace_::Mutex*)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/base/mutex.h:290
(libglog.so.0+0x1abe7)
#3 google::InitVLOG3__(int**, int*, char const*, int)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/vlog_is_on.cc:199
(libglog.so.0+0x1abe7)
#4
kudu::rpc::Messenger::ShutdownInternal(kudu::rpc::Messenger::ShutdownMode)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:283:5
(libkrpc.so+0xab101)
#5 kudu::rpc::Messenger::AllExternalReferencesDropped()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:249:3
(libkrpc.so+0xaaeb7)
#6 std::__1::mem_fun_t<void,
kudu::rpc::Messenger>::operator()(kudu::rpc::Messenger*) const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1120:17
(libkrpc.so+0xaf3a5)
#7 std::__1::__shared_ptr_pointer<kudu::rpc::Messenger*,
std::__1::mem_fun_t<void, kudu::rpc::Messenger>,
std::__1::allocator<kudu::rpc::Messenger> >::__on_zero_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkrpc.so+0xaf3a5)
#8 std::__1::__shared_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x56affe)
#9 std::__1::__shared_weak_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x56affe)
#10 std::__1::shared_ptr<kudu::rpc::Messenger>::~shared_ptr()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x56affe)
#11 kudu::client::KuduClient::Data::~Data()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client-internal.cc:179:1
(libkudu_client.so+0x136260)
#12 kudu::client::KuduClient::~KuduClient()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client.cc:394:3
(libkudu_client.so+0x1130cc)
#13
std::__1::default_delete<kudu::client::KuduClient>::operator()(kudu::client::KuduClient*)
const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:2285:5
(libkudu_client.so+0x12be1b)
#14 std::__1::__shared_ptr_pointer<kudu::client::KuduClient*,
std::__1::default_delete<kudu::client::KuduClient>,
std::__1::allocator<kudu::client::KuduClient> >::__on_zero_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkudu_client.so+0x12be1b)
#15 std::__1::__shared_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x550d1e)
#16 std::__1::__shared_weak_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x550d1e)
#17 std::__1::shared_ptr<kudu::client::KuduClient>::~shared_ptr()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x550d1e)
#18 kudu::tools::LeaderMasterProxy::~LeaderMasterProxy()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.h:233:7
(kudu+0x576cf9)
#19 kudu::tools::(anonymous
namespace)::ListMasters(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_master.cc:180:1
(kudu+0x572d0b)
#20
_ZNSt3__18__invokeIRPFN4kudu6StatusERKNS1_5tools13RunnerContextEEJS6_EEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOSA_DpOSB_
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/type_traits:4482:1
(kudu+0x52e48b)
#21 kudu::Status
std::__1::__invoke_void_return_wrapper<kudu::Status>::__call<kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext
const&>(kudu::Status (*&)(kudu::tools::RunnerContext const&),
kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__functional_base:318
(kudu+0x52e48b)
#22 std::__1::__function::__func<kudu::Status
(*)(kudu::tools::RunnerContext const&), std::__1::allocator<kudu::Status
(*)(kudu::tools::RunnerContext const&)>, kudu::Status
(kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext
const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1562:12
(kudu+0x52e3bd)
#23 std::__1::function<kudu::Status (kudu::tools::RunnerContext
const&)>::operator()(kudu::tools::RunnerContext const&) const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1916:12
(libkudu_tools_util.so+0x6c1c4)
#24 kudu::tools::Action::Run(std::__1::vector<kudu::tools::Mode*,
std::__1::allocator<kudu::tools::Mode*> > const&,
std::__1::unordered_map<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::hash<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > >,
std::__1::equal_to<std::__1::basic_string<char, std::__1::char_trai [...]
#25 kudu::tools::DispatchCommand(std::__1::vector<kudu::tools::Mode*,
std::__1::allocator<kudu::tools::Mode*> > const&, kudu::tools::Action*,
std::__1::deque<std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:132:15
(kudu+0x5b42b6)
#26 kudu::tools::RunTool(int, char**, bool)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:204:16
(kudu+0x5b5211)
#27 main
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:265:10
(kudu+0x5b557e)
Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative
warning message
Mutex M1870 acquired here while holding mutex M37857528269694952 in
thread T8:
#0 AnnotateRWLockAcquired
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cc:271
(kudu+0x4d53ff)
#1 kudu::rw_spinlock::lock()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/locks.h:112:5
(libkudu_client.so+0x177762)
#2 kudu::percpu_rwlock::lock()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/locks.h:222:22
(libkudu_client.so+0x1776f2)
#3
std::__1::lock_guard<kudu::percpu_rwlock>::lock_guard(kudu::percpu_rwlock&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__mutex_base:104:27
(libkrpc.so+0xac9c9)
#4 kudu::rpc::Messenger::~Messenger()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:430
(libkrpc.so+0xac9c9)
#5
std::__1::default_delete<kudu::rpc::Messenger>::operator()(kudu::rpc::Messenger*)
const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:2285:5
(libkrpc.so+0xb246b)
#6 std::__1::__shared_ptr_pointer<kudu::rpc::Messenger*,
std::__1::default_delete<kudu::rpc::Messenger>,
std::__1::allocator<kudu::rpc::Messenger> >::__on_zero_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkrpc.so+0xb246b)
#7 std::__1::__shared_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x56affe)
#8 std::__1::__shared_weak_count::__release_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x56affe)
#9 std::__1::shared_ptr<kudu::rpc::Messenger>::~shared_ptr()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x56affe)
#10 std::__1::shared_ptr<kudu::rpc::Messenger>::reset()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4603:5
(libkrpc.so+0xc0771)
#11 kudu::rpc::ReactorThread::RunThread()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:499
(libkrpc.so+0xc0771)
#12 boost::_mfi::mf0<void,
kudu::rpc::ReactorThread>::operator()(kudu::rpc::ReactorThread*) const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/mem_fn_template.hpp:49:29
(libkrpc.so+0xca669)
#13 void boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*>
>::operator()<boost::_mfi::mf0<void, kudu::rpc::ReactorThread>,
boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf0<void,
kudu::rpc::ReactorThread>&, boost::_bi::list0&, int)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/bind.hpp:259:9
(libkrpc.so+0xca5ba)
#14 boost::_bi::bind_t<void, boost::_mfi::mf0<void,
kudu::rpc::ReactorThread>,
boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*> >
>::operator()()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/bind.hpp:1222:16
(libkrpc.so+0xca543)
#15
boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void,
boost::_mfi::mf0<void, kudu::rpc::ReactorThread>,
boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*> > >,
void>::invoke(boost::detail::function::function_buffer&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/function/function_template.hpp:159:11
(libkrpc.so+0xca339)
#16 boost::function0<void>::operator()() const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/function/function_template.hpp:770:14
(libkrpc.so+0xba0b1)
#17 kudu::Thread::SuperviseThread(void*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.cc:657:3
(libkudu_util.so+0x1ee174)
Thread T8 'rpc reactor-588' (tid=5886, running) created by main thread at:
#0 pthread_create
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:992
(kudu+0x490e36)
#1 kudu::Thread::StartThread(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > const&,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > const&, boost::function<void ()> const&, unsigned
long, scoped_refptr<kudu::Thread>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.cc:601:15
(libkudu_util.so+0x1ed95b)
#2 kudu::Status kudu::Thread::Create<void
(kudu::rpc::ReactorThread::*)(),
kudu::rpc::ReactorThread*>(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > const&,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > const&, void (kudu::rpc::ReactorThread::*
const&)(), kudu::rpc::ReactorThread* const&, scoped_refptr<kudu::Thread>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.h:164:12
(libkrpc.so+ [...]
#3 kudu::rpc::ReactorThread::Init()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:185:10
(libkrpc.so+0xc026e)
#4 kudu::rpc::Reactor::Init()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:759:18
(libkrpc.so+0xc4911)
#5 kudu::rpc::Messenger::Init()
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:446:5
(libkrpc.so+0xaad72)
#6
kudu::rpc::MessengerBuilder::Build(std::__1::shared_ptr<kudu::rpc::Messenger>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:205:3
(libkrpc.so+0xaa7cd)
#7
kudu::client::KuduClientBuilder::Build(std::__1::shared_ptr<kudu::client::KuduClient>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client.cc:349:3
(libkudu_client.so+0x112561)
#8
kudu::tools::LeaderMasterProxy::Init(std::__1::vector<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >,
std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > > > const&, kudu::MonoDelta const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.cc:786:30
(libkudu_tools_util.so+0x7740c)
#9 kudu::tools::LeaderMasterProxy::Init(kudu::tools::RunnerContext
const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.cc:792:10
(libkudu_tools_util.so+0x774d6)
#10 kudu::tools::(anonymous
namespace)::ListMasters(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_master.cc:109:3
(kudu+0x572be3)
#11
_ZNSt3__18__invokeIRPFN4kudu6StatusERKNS1_5tools13RunnerContextEEJS6_EEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOSA_DpOSB_
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/type_traits:4482:1
(kudu+0x52e48b)
#12 kudu::Status
std::__1::__invoke_void_return_wrapper<kudu::Status>::__call<kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext
const&>(kudu::Status (*&)(kudu::tools::RunnerContext const&),
kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__functional_base:318
(kudu+0x52e48b)
#13 std::__1::__function::__func<kudu::Status
(*)(kudu::tools::RunnerContext const&), std::__1::allocator<kudu::Status
(*)(kudu::tools::RunnerContext const&)>, kudu::Status
(kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext
const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1562:12
(kudu+0x52e3bd)
#14 std::__1::function<kudu::Status (kudu::tools::RunnerContext
const&)>::operator()(kudu::tools::RunnerContext const&) const
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1916:12
(libkudu_tools_util.so+0x6c1c4)
#15 kudu::tools::Action::Run(std::__1::vector<kudu::tools::Mode*,
std::__1::allocator<kudu::tools::Mode*> > const&,
std::__1::unordered_map<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::hash<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > >,
std::__1::equal_to<std::__1::basic_string<char, std::__1::char_trai [...]
#16 kudu::tools::DispatchCommand(std::__1::vector<kudu::tools::Mode*,
std::__1::allocator<kudu::tools::Mode*> > const&, kudu::tools::Action*,
std::__1::deque<std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > > > const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:132:15
(kudu+0x5b42b6)
#17 kudu::tools::RunTool(int, char**, bool)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:204:16
(kudu+0x5b5211)
#18 main
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:265:10
(kudu+0x5b557e)
Change-Id: I1fd93c06b14bc97a9ac4a37a5b6ca55ffa38f544
Reviewed-on: http://gerrit.cloudera.org:8080/14250
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/rpc/messenger.cc | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 4129172..871192a 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -372,9 +372,7 @@ void Messenger::QueueOutboundCall(const
shared_ptr<OutboundCall> &call) {
}
void Messenger::QueueInboundCall(gscoped_ptr<InboundCall> call) {
- shared_lock<rw_spinlock> guard(lock_.get_lock());
- scoped_refptr<RpcService>* service = FindOrNull(rpc_services_,
-
call->remote_method().service_name());
+ const auto service = rpc_service(call->remote_method().service_name());
if (PREDICT_FALSE(!service)) {
Status s = Status::ServiceUnavailable(Substitute("service $0 not
registered on $1",
call->remote_method().service_name(), name_));
@@ -383,10 +381,10 @@ void Messenger::QueueInboundCall(gscoped_ptr<InboundCall>
call) {
return;
}
- call->set_method_info((*service)->LookupMethod(call->remote_method()));
+ call->set_method_info(service->LookupMethod(call->remote_method()));
// The RpcService will respond to the client on success or failure.
- WARN_NOT_OK((*service)->QueueInboundCall(std::move(call)), "Unable to handle
RPC call");
+ WARN_NOT_OK(service->QueueInboundCall(std::move(call)), "Unable to handle
RPC call");
}
void Messenger::QueueCancellation(const shared_ptr<OutboundCall> &call) {
@@ -427,7 +425,6 @@ Messenger::Messenger(const MessengerBuilder &bld)
}
Messenger::~Messenger() {
- std::lock_guard<percpu_rwlock> guard(lock_);
CHECK(closing_) << "Should have already shut down";
STLDeleteElements(&reactors_);
}
@@ -451,7 +448,6 @@ Status Messenger::Init() {
Status Messenger::DumpConnections(const DumpConnectionsRequestPB& req,
DumpConnectionsResponsePB* resp) {
- shared_lock<rw_spinlock> guard(lock_.get_lock());
for (Reactor* reactor : reactors_) {
RETURN_NOT_OK(reactor->DumpConnections(req, resp));
}
