From: Adam C. Emerson Date: Thu, 2 Feb 2023 06:14:35 +0000 (-0500) Subject: osdc: Catch exceptions thrown in CLS client decoders X-Git-Tag: v19.3.0~349^2~32 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=af192bf800006fb484cca388aa909193c016f803;p=ceph.git osdc: Catch exceptions thrown in CLS client decoders And return them to the client by setting the error code and result in the vector and returning an error from the operation as a whole. Pass OSD failure to subsequent handlers so that, for example, in the event of a cancellation, we don't try to decode data that isn't there. Signed-off-by: Adam C. Emerson --- diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index f006597e8273..6bc713fcbf5e 100644 --- a/src/osdc/Objecter.cc +++ b/src/osdc/Objecter.cc @@ -3537,11 +3537,23 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m) ceph_assert(op->out_bl.size() == op->out_rval.size()); ceph_assert(op->out_bl.size() == op->out_handler.size()); auto p = out_ops.begin(); + // Propagates handler error to Op::completion. In the event of + // multiple handler errors, the most recent wins. + bs::error_code handler_error; + // Holds OSD error code, so handlers downstream of a failing op are + // made aware of it. + bs::error_code first_osd_error; for (unsigned i = 0; p != out_ops.end() && pb != op->out_bl.end(); ++i, ++p, ++pb, ++pr, ++pe, ++ph) { ldout(cct, 10) << " op " << i << " rval " << p->rval << " len " << p->outdata.length() << dendl; + // Track when we get an OSD error and supply it to subsequent + // handlers so they won't attempt to operate on data that isn't + // there. + if (!first_osd_error && (p->rval < 0)) { + first_osd_error = bs::error_code(-p->rval, osd_category()); + } if (*pb) **pb = p->outdata; // set rval before running handlers so that handlers @@ -3552,10 +3564,35 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m) **pe = p->rval < 0 ? bs::error_code(-p->rval, osd_category()) : bs::error_code(); if (*ph) { - std::move((*ph))(p->rval < 0 ? - bs::error_code(-p->rval, osd_category()) : - bs::error_code(), - p->rval, p->outdata); + try { + bs::error_code e; + if (first_osd_error) { + e = first_osd_error; + } else if (p->rval < 0) { + e = bs::error_code(-p->rval, osd_category()); + } + std::move((*ph))(e, p->rval, p->outdata); + } catch (const bs::system_error& e) { + ldout(cct, 10) << "ERROR: tid " << op->tid << ": handler function threw " + << e.what() << dendl; + handler_error = e.code(); + if (*pe) { + **pe = e.code(); + } + if (*pr) { + **pr = ceph::from_error_code(e.code()); + } + } catch (const std::exception& e) { + ldout(cct, 0) << "ERROR: tid " << op->tid << ": handler function threw " + << e.what() << dendl; + handler_error = osdc_errc::handler_failed; + if (*pe) { + **pe = osdc_errc::handler_failed; + } + if (*pr) { + **pr = -EIO; + } + } } } @@ -3587,7 +3624,11 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m) // do callbacks if (Op::has_completion(onfinish)) { - Op::complete(std::move(onfinish), osdcode(rc), rc); + if (rc == 0 && handler_error) { + Op::complete(std::move(onfinish), handler_error, -EIO); + } else { + Op::complete(std::move(onfinish), osdcode(rc), rc); + } } if (completion_lock.mutex()) { completion_lock.unlock(); diff --git a/src/osdc/error_code.cc b/src/osdc/error_code.cc index 7823e8b088c4..d60faa587bcc 100644 --- a/src/osdc/error_code.cc +++ b/src/osdc/error_code.cc @@ -73,6 +73,9 @@ const char* osdc_error_category::message(int ev, char*, case osdc_errc::pool_eio: return "Pool EIO flag set"; + + case osdc_errc::handler_failed: + return "Handler function threw unknown exception"; } return "Unknown error"; @@ -101,6 +104,8 @@ osdc_error_category::default_error_condition(int ev) const noexcept { return bs::errc::timed_out; case osdc_errc::pool_eio: return bs::errc::io_error; + case osdc_errc::handler_failed: + return bs::errc::io_error; } return { ev, *this }; @@ -156,6 +161,8 @@ int osdc_error_category::from_code(int ev) const noexcept { return -ETIMEDOUT; case osdc_errc::pool_eio: return -EIO; + case osdc_errc::handler_failed: + return -EIO; } return -EDOM; } diff --git a/src/osdc/error_code.h b/src/osdc/error_code.h index eb78a5110b01..88d6f080a8a4 100644 --- a/src/osdc/error_code.h +++ b/src/osdc/error_code.h @@ -30,7 +30,8 @@ enum class osdc_errc { snapshot_exists, snapshot_dne, timed_out, - pool_eio + pool_eio, + handler_failed }; namespace boost::system { diff --git a/src/test/neorados/CMakeLists.txt b/src/test/neorados/CMakeLists.txt index 023f6580397f..c625133d50a5 100644 --- a/src/test/neorados/CMakeLists.txt +++ b/src/test/neorados/CMakeLists.txt @@ -1,4 +1,3 @@ - add_executable(ceph_test_neorados test_neorados.cc) target_link_libraries(ceph_test_neorados global libneorados ${unittest_libs} @@ -26,3 +25,19 @@ target_link_libraries(neoradostest-support add_executable(ceph_test_neorados_list_pool list_pool.cc) target_link_libraries(ceph_test_neorados_list_pool libneorados neoradostest-support global ${FMT_LIB} ${unittest_libs}) + +add_executable(ceph_test_neorados_handler_error + handler_error.cc + ) +target_link_libraries(ceph_test_neorados_handler_error + libneorados + ${BLKID_LIBRARIES} + ${CMAKE_DL_LIBS} + ${CRYPTO_LIBS} + ${EXTRALIBS} + neoradostest-support + ${UNITTEST_LIBS} + ) +install(TARGETS + ceph_test_neorados_handler_error + DESTINATION ${CMAKE_INSTALL_BINDIR}) diff --git a/src/test/neorados/handler_error.cc b/src/test/neorados/handler_error.cc new file mode 100644 index 000000000000..26d468bc0246 --- /dev/null +++ b/src/test/neorados/handler_error.cc @@ -0,0 +1,62 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2023 IBM + * + * See file COPYING for license information. + * + */ + +#include +#include +#include +#include + +#include +#include + +#include +#include + +#include "include/neorados/RADOS.hpp" + +#include "cls/version/cls_version_types.h" + +#include "test/neorados/common_tests.h" + +#include "gtest/gtest.h" + +namespace asio = boost::asio; +namespace sys = boost::system; +namespace buffer = ceph::buffer; + +CORO_TEST_F(neocls_handler_error, test_handler_error, NeoRadosTest) +{ + std::string_view oid = "obj"; + co_await create_obj(rados(), oid, pool(), asio::use_awaitable); + + { + neorados::ReadOp op; + op.exec("version", "read", {}, + [](sys::error_code ec, const buffer::list& bl) { + throw buffer::end_of_buffer{}; + }); + co_await expect_error_code(rados().execute(oid, pool(), std::move(op), + nullptr, asio::use_awaitable), + buffer::errc::end_of_buffer); + } + + { + neorados::ReadOp op; + op.exec("version", "read", {}, + [](sys::error_code ec, const buffer::list& bl) { + throw std::exception(); + }); + co_await expect_error_code(rados().execute(oid, pool(), std::move(op), + nullptr, asio::use_awaitable), + sys::errc::io_error); + } + co_return; +}