You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2017/04/22 07:40:09 UTC

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Jim Apple has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6714

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................

IMPALA-5031: remove undefined behavior: nullptr member function call

This member function call on a nullptr to a FragmentInstanceState was
present in data loading, end-to-end tests, and custom cluster tests.

Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
---
M be/src/runtime/coordinator.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/6714/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

I suppose all current callers of this function are from code path with coordinator fragment and that's why we haven't hit any crash yet ?

http://gerrit.cloudera.org:8080/#/c/6714/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS1, Line 379: coord_instance_ == nullptr)
parenthesis is not needed.


-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6714/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 379:   return (coord_instance_ == nullptr) ? nullptr : coord_instance_->executor();
While I agree they are not needed, I find it to be clearer this way.

Unrelatedly, here is a sample backtrace:

    /home/ubuntu/Impala/be/src/runtime/fragment-instance-state.h:74:46: runtime error: member access within null pointer of type 'impala::FragmentInstanceState'
    #0 0x7fcdea60aaf8 in impala::FragmentInstanceState::executor() /home/ubuntu/Impala/be/src/runtime/fragment-instance-state.h:74:46
    #1 0x7fcdea5a6239 in impala::Coordinator::executor() /home/ubuntu/Impala/be/src/runtime/coordinator.cc:379:10
    #2 0x7fcdea5b639b in impala::Coordinator::PrintFragmentInstanceInfo() /home/ubuntu/Impala/be/src/runtime/coordinator.cc:1143:18
    #3 0x7fcdea5acdb8 in impala::Coordinator::Exec() /home/ubuntu/Impala/be/src/runtime/coordinator.cc:507:3
    #4 0x7fcde40f110f in impala::ImpalaServer::QueryExecState::ExecQueryOrDmlRequest(impala::TQueryExecRequest const&) /home/ubuntu/Impala/be/src/service/query-exec-state.cc:469:12
    #5 0x7fcde40e6835 in impala::ImpalaServer::QueryExecState::Exec(impala::TExecRequest*) /home/ubuntu/Impala/be/src/service/query-exec-state.cc:158:14
    #6 0x7fcde3cf04fc in impala::ImpalaServer::ExecuteInternal(impala::TQueryCtx const&, std::shared_ptr<impala::ImpalaServer::SessionState>, bool*, std::shared_ptr<impala::ImpalaServer::QueryExecState>*) /home/ubuntu/Impala/be/src/service/impala-server.cc:829:29
    #7 0x7fcde3cecd82 in impala::ImpalaServer::Execute(impala::TQueryCtx*, std::shared_ptr<impala::ImpalaServer::SessionState>, std::shared_ptr<impala::ImpalaServer::QueryExecState>*) /home/ubuntu/Impala/be/src/service/impala-server.cc:775:19
    #8 0x7fcde4098e60 in impala::ImpalaServer::query(beeswax::QueryHandle&, beeswax::Query const&) /home/ubuntu/Impala/be/src/service/impala-beeswax-server.cc:68:29
    #9 0x7fcde13efdd7 in beeswax::BeeswaxServiceProcessor::process_query(int, apache::thrift::protocol::TProtocol*, apache::thrift::protocol::TProtocol*, void*) /home/ubuntu/Impala/be/generated-sources/gen-cpp/BeeswaxService.cpp:2979:5
    #10 0x7fcde13ef165 in beeswax::BeeswaxServiceProcessor::dispatchCall(apache::thrift::protocol::TProtocol*, apache::thrift::protocol::TProtocol*, std::string const&, int, void*) /home/ubuntu/Impala/be/generated-sources/gen-cpp/BeeswaxService.cpp:2952:3
    #11 0x7fcde1347c16 in impala::ImpalaServiceProcessor::dispatchCall(apache::thrift::protocol::TProtocol*, apache::thrift::protocol::TProtocol*, std::string const&, int, void*) /home/ubuntu/Impala/be/generated-sources/gen-cpp/ImpalaService.cpp:1673:12
    #12 0x7fcdeccdb48e in apache::thrift::TDispatchProcessor::process(boost::shared_ptr<apache::thrift::protocol::TProtocol>, boost::shared_ptr<apache::thrift::protocol::TProtocol>, void*) /home/ubuntu/Impala/toolchain/thrift-0.9.0-p8/include/thrift/TDispatchProcessor.h:121:12
    #13 0x17e40ea in apache::thrift::server::TThreadPoolServer::Task::run() (/home/ubuntu/Impala/be/build/debug/service/impalad+0x17e40ea)
    #14 0x17c75f8 in apache::thrift::concurrency::ThreadManager::Worker::run() (/home/ubuntu/Impala/be/build/debug/service/impalad+0x17c75f8)
    #15 0x7fcde4ee2a20 in impala::ThriftThread::RunRunnable(boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*) /home/ubuntu/Impala/be/src/rpc/thrift-thread.cc:64:3
    #16 0x7fcde4ee916b in boost::_mfi::mf2<void, impala::ThriftThread, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*>::operator()(impala::ThriftThread*, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*) const /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/mem_fn_template.hpp:280:16
    #17 0x7fcde4ee8e8a in void boost::_bi::list3<boost::_bi::value<impala::ThriftThread*>, boost::_bi::value<boost::shared_ptr<apache::thrift::concurrency::Runnable> >, boost::_bi::value<impala::Promise<unsigned long>*> >::operator()<boost::_mfi::mf2<void, impala::ThriftThread, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf2<void, impala::ThriftThread, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*>&, boost::_bi::list0&, int) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:392:9
    #18 0x7fcde4ee894b in boost::_bi::bind_t<void, boost::_mfi::mf2<void, impala::ThriftThread, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*>, boost::_bi::list3<boost::_bi::value<impala::ThriftThread*>, boost::_bi::value<boost::shared_ptr<apache::thrift::concurrency::Runnable> >, boost::_bi::value<impala::Promise<unsigned long>*> > >::operator()() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind_template.hpp:20:16
    #19 0x7fcde4ee7c09 in boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf2<void, impala::ThriftThread, boost::shared_ptr<apache::thrift::concurrency::Runnable>, impala::Promise<unsigned long>*>, boost::_bi::list3<boost::_bi::value<impala::ThriftThread*>, boost::_bi::value<boost::shared_ptr<apache::thrift::concurrency::Runnable> >, boost::_bi::value<impala::Promise<unsigned long>*> > >, void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:153:11
    #20 0x7fcdec4a6dd4 in boost::function0<void>::operator()() const /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:766:14
    #21 0x7fcdec497f10 in impala::Thread::SuperviseThread(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*) /home/ubuntu/Impala/be/src/util/thread.cc:325:3
    #22 0x7fcdec4bff83 in void boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> >::operator()<void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list0>(boost::_bi::type<void>, void (*&)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list0&, int) /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:457:9
    #23 0x7fcdec4bf8ab in boost::_bi::bind_t<void, void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> > >::operator()() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind_template.hpp:20:16
    #24 0x7fcdec4be2f5 in boost::detail::thread_data<boost::_bi::bind_t<void, void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::Promise<long>*), boost::_bi::list4<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::Promise<long>*> > > >::run() /home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/thread/detail/thread.hpp:116:17
    #25 0xa4d0b9 in thread_proxy (/home/ubuntu/Impala/be/build/debug/service/impalad+0xa4d0b9)
    #26 0x7fcddcf18183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183)
    #27 0x7fcddcc4537c in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfa37c)


More can be found in the logs of this Jenkins job: http://jenkins.impala.io:8080/job/ubuntu-14.04-from-scratch/1173/artifact/Impala/logs_static/logs/


-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................


IMPALA-5031: remove undefined behavior: nullptr member function call

This member function call on a nullptr to a FragmentInstanceState was
present in data loading, end-to-end tests, and custom cluster tests.

Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Reviewed-on: http://gerrit.cloudera.org:8080/6714
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/coordinator.cc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Verified
  Michael Ho: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: nullptr member function call

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: nullptr member function call
......................................................................


Patch Set 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/496/

-- 
To view, visit http://gerrit.cloudera.org:8080/6714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2377c14f7ea8f93bb96504dd2319c11ff709cd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: No