Skip to content

Commit c050cc8

Browse files
adamemersonadamemerson
committed
rgw: Update run_coro to use redirect_error and add tests
Should have added tests the first time, but better late than never. Signed-off-by: Adam C. Emerson <[email protected]>
1 parent c1d9a26 commit c050cc8

File tree

3 files changed

+265
-58
lines changed

3 files changed

+265
-58
lines changed

‎src/rgw/async_utils.h‎

Lines changed: 55 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "common/async/blocked_completion.h"
3030
#include "common/async/concepts.h"
3131
#include "common/async/yield_context.h"
32+
#include "common/async/redirect_error.h"
3233

3334
#include "common/dout.h"
3435
#include "common/dout_fmt.h"
@@ -60,13 +61,10 @@ inline int run_coro(
6061
std::string* what ///< Where to store the result of `what()` on exception
6162
) noexcept
6263
{
63-
try {
64-
maybe_warn_about_blocking(dpp);
65-
asio::co_spawn(executor, std::move(coro), async::use_blocked);
66-
} catch (const std::exception&) {
67-
return ceph::from_exception(std::current_exception(), what);
68-
}
69-
return 0;
64+
std::exception_ptr e;
65+
maybe_warn_about_blocking(dpp);
66+
asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
67+
return ceph::from_exception(e, what);
7068
}
7169

7270
/// Call a coroutine and block until it completes, handling exceptions
@@ -107,13 +105,10 @@ int run_coro(
107105
std::string* what ///< Where to store the result of `what()`.
108106
) noexcept
109107
{
110-
try {
111-
val = asio::co_spawn(executor, std::move(coro), async::use_blocked);
112-
maybe_warn_about_blocking(dpp);
113-
} catch (const std::exception& e) {
114-
return ceph::from_exception(std::current_exception(), what);
115-
}
116-
return 0;
108+
std::exception_ptr e;
109+
maybe_warn_about_blocking(dpp);
110+
val = asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
111+
return ceph::from_exception(e, what);
117112
}
118113

119114
/// Call a coroutine and block until it completes, handling exceptions
@@ -156,13 +151,10 @@ int run_coro(
156151
std::string* what ///< Where to store the result of `what()`.
157152
) noexcept
158153
{
159-
try {
160-
maybe_warn_about_blocking(dpp);
161-
vals = asio::co_spawn(executor, std::move(coro), async::use_blocked);
162-
} catch (const std::exception& e) {
163-
return ceph::from_exception(std::current_exception(), what);
164-
}
165-
return 0;
154+
std::exception_ptr e;
155+
maybe_warn_about_blocking(dpp);
156+
vals = asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
157+
return ceph::from_exception(e, what);
166158
}
167159

168160
/// Call a coroutine and block until it completes, handling exceptions
@@ -203,19 +195,21 @@ int run_coro(
203195
int log_level = 5 /// What level to log at
204196
) noexcept
205197
{
206-
try {
207-
if (y) {
208-
auto& yield = y.get_yield_context();
209-
asio::co_spawn(yield.get_executor(), std::move(coro), yield);
210-
} else {
211-
maybe_warn_about_blocking(dpp);
212-
asio::co_spawn(executor, std::move(coro), async::use_blocked);
213-
}
214-
} catch (const std::exception& e) {
215-
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, e.what());
216-
return ceph::from_exception(std::current_exception());
198+
std::exception_ptr e;
199+
if (y) {
200+
auto& yield = y.get_yield_context();
201+
asio::co_spawn(yield.get_executor(), std::move(coro),
202+
async::redirect_error(yield, e));
203+
} else {
204+
maybe_warn_about_blocking(dpp);
205+
asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
217206
}
218-
return 0;
207+
std::string what;
208+
auto r = ceph::from_exception(e, &what);
209+
if (e) [[unlikely]] {
210+
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, what);
211+
}
212+
return r;
219213
}
220214

221215
/// Call a coroutine and block until it completes, handling exceptions
@@ -260,20 +254,21 @@ int run_coro(
260254
int log_level = 5 /// What level to log at
261255
) noexcept
262256
{
263-
try {
264-
if (y) {
265-
auto& yield = y.get_yield_context();
266-
val = asio::co_spawn(yield.get_executor(), std::move(coro), yield);
267-
} else {
268-
maybe_warn_about_blocking(dpp);
269-
val = asio::co_spawn(executor, std::move(coro), async::use_blocked);
270-
}
271-
} catch (const std::exception& e) {
272-
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, e.what());
273-
return ceph::from_exception(std::current_exception());
257+
std::exception_ptr e;
258+
if (y) {
259+
auto& yield = y.get_yield_context();
260+
val = asio::co_spawn(yield.get_executor(), std::move(coro),
261+
async::redirect_error(yield, e));
262+
} else {
263+
maybe_warn_about_blocking(dpp);
264+
val = asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
274265
}
275-
276-
return 0;
266+
std::string what;
267+
auto r = ceph::from_exception(e, &what);
268+
if (e) [[unlikely]] {
269+
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, what);
270+
}
271+
return r;
277272
}
278273

279274
/// Call a coroutine and block until it completes, handling exceptions
@@ -317,19 +312,21 @@ int run_coro(
317312
int log_level = 5 /// What level to log at
318313
) noexcept
319314
{
320-
try {
321-
if (y) {
322-
auto& yield = y.get_yield_context();
323-
vals = asio::co_spawn(yield.get_executor(), std::move(coro), yield);
324-
} else {
325-
maybe_warn_about_blocking(dpp);
326-
vals = asio::co_spawn(executor, std::move(coro), async::use_blocked);
327-
}
328-
} catch (const std::exception& e) {
329-
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, e.what());
330-
return ceph::from_exception(std::current_exception());
315+
std::exception_ptr e;
316+
if (y) {
317+
auto& yield = y.get_yield_context();
318+
vals = asio::co_spawn(yield.get_executor(), std::move(coro),
319+
async::redirect_error(yield, e));
320+
} else {
321+
maybe_warn_about_blocking(dpp);
322+
vals = asio::co_spawn(executor, std::move(coro), async::use_blocked[e]);
323+
}
324+
std::string what;
325+
auto r = ceph::from_exception(e, &what);
326+
if (e) [[unlikely]] {
327+
ldpp_dout_fmt(dpp, log_level, "{}: failed: {}", name, what);
331328
}
332-
return 0;
329+
return r;
333330
}
334331

335332
/// Call a coroutine and block until it completes, handling exceptions

‎src/test/rgw/CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,6 @@ endfunction()
416416

417417
add_catch2_test_rgw(rgw_hex)
418418

419+
add_executable(unittest_rgw_async_utils test_rgw_async_utils.cc)
420+
add_ceph_unittest(unittest_rgw_async_utils)
421+
target_link_libraries(unittest_rgw_async_utils ${rgw_libs} ${UNITTEST_LIBS})

‎src/test/rgw/test_rgw_async_utils.cc‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
2+
// vim: ts=8 sw=2 smarttab ft=cpp
3+
4+
/*
5+
* Ceph - scalable distributed file system
6+
*
7+
* Copyright contributors to the Ceph project
8+
*
9+
* This is free software; you can redistribute it and/or
10+
* modify it under the terms of the GNU Lesser General Public
11+
* License version 2.1, as published by the Free Software
12+
* Foundation. See file COPYING.
13+
*
14+
*/
15+
16+
#include "rgw/async_utils.h"
17+
18+
#include <cerrno>
19+
#include <memory>
20+
#include <string>
21+
#include <tuple>
22+
23+
#include <boost/asio/awaitable.hpp>
24+
#include <boost/asio/this_coro.hpp>
25+
#include <boost/asio/spawn.hpp>
26+
27+
#include <boost/system/generic_category.hpp>
28+
#include <boost/system/system_error.hpp>
29+
30+
#include <gtest/gtest.h>
31+
32+
#include "common/async/context_pool.h"
33+
#include "common/async/yield_context.h"
34+
35+
#include <common/ceph_context.h>
36+
#include <common/dout.h>
37+
38+
namespace asio = boost::asio;
39+
namespace async = ceph::async;
40+
namespace sys = boost::system;
41+
42+
static auto cct = std::make_unique<CephContext>(CEPH_ENTITY_TYPE_ANY);
43+
44+
static NoDoutPrefix dp(cct.get(), ceph_subsys_rgw);
45+
46+
TEST(CoSpawn, CoSpawn) {
47+
async::io_context_pool pool{3};
48+
49+
auto maybethrow = [](int code) -> asio::awaitable<void> {
50+
if (code != 0) {
51+
throw sys::system_error{code, sys::generic_category()};
52+
}
53+
co_return;
54+
};
55+
56+
int r = 0;
57+
r = rgw::run_coro(&dp, pool, maybethrow(ENOENT), nullptr);
58+
ASSERT_EQ(-ENOENT, r);
59+
60+
r = rgw::run_coro(&dp, pool, maybethrow(0), nullptr);
61+
ASSERT_EQ(0, r);
62+
63+
asio::spawn(pool,
64+
[&](asio::yield_context y) -> void {
65+
r = rgw::run_coro(&dp, pool, maybethrow(ENOENT), "yielding",
66+
y);
67+
},
68+
async::use_blocked);
69+
ASSERT_EQ(-ENOENT, r);
70+
71+
asio::spawn(pool,
72+
[&](asio::yield_context y) -> void {
73+
r = rgw::run_coro(&dp, pool, maybethrow(0), "yielding",
74+
y);
75+
},
76+
async::use_blocked);
77+
ASSERT_EQ(0, r);
78+
79+
r = rgw::run_coro(&dp, pool, maybethrow(ENOENT), "blocking",
80+
null_yield);
81+
ASSERT_EQ(-ENOENT, r);
82+
83+
r = rgw::run_coro(&dp, pool, maybethrow(0), "blocking",
84+
null_yield);
85+
ASSERT_EQ(0, r);
86+
87+
auto maybethrowv = []<typename V>(int code, V v) -> asio::awaitable<V> {
88+
if (code != 0) {
89+
throw sys::system_error{code, sys::generic_category()};
90+
}
91+
co_return std::move(v);
92+
};
93+
94+
const std::string instr("foo");
95+
std::string s;
96+
97+
r = rgw::run_coro(&dp, pool, maybethrowv(ENOENT, instr),
98+
s, nullptr);
99+
ASSERT_EQ(-ENOENT, r);
100+
ASSERT_TRUE(s.empty());
101+
102+
r = rgw::run_coro(&dp, pool, maybethrowv(0, instr), s, nullptr);
103+
ASSERT_EQ(0, r);
104+
ASSERT_EQ(instr, s);
105+
106+
s.clear();
107+
108+
asio::spawn(pool,
109+
[&](asio::yield_context y) -> void {
110+
r = rgw::run_coro(&dp, pool, maybethrowv(ENOENT, instr),
111+
s, "yielding", y);
112+
},
113+
async::use_blocked);
114+
ASSERT_EQ(-ENOENT, r);
115+
ASSERT_TRUE(s.empty());
116+
117+
asio::spawn(pool,
118+
[&](asio::yield_context y) -> void {
119+
r = rgw::run_coro(&dp, pool, maybethrowv(0, instr),
120+
s, "yielding", y);
121+
},
122+
async::use_blocked);
123+
ASSERT_EQ(0, r);
124+
ASSERT_EQ(instr, s);
125+
126+
s.clear();
127+
r = rgw::run_coro(&dp, pool, maybethrowv(ENOENT, instr), s,
128+
"blocking", null_yield);
129+
ASSERT_EQ(-ENOENT, r);
130+
ASSERT_TRUE(s.empty());
131+
132+
r = rgw::run_coro(&dp, pool, maybethrowv(0, instr), s,
133+
"blocking", null_yield);
134+
ASSERT_EQ(0, r);
135+
ASSERT_EQ(instr, s);
136+
137+
auto maybethrowvs = []<typename ...Vs>(int code, Vs ...vs)
138+
-> asio::awaitable<std::tuple<Vs...>> {
139+
if (code != 0) {
140+
throw sys::system_error{code, sys::generic_category()};
141+
}
142+
co_return std::make_tuple(std::move(vs)...);
143+
};
144+
145+
s.clear();
146+
std::unique_ptr<int> p;
147+
148+
r = rgw::run_coro(&dp, pool, maybethrowvs(ENOENT, instr,
149+
std::make_unique<int>(5)),
150+
std::tie(s, p), nullptr);
151+
ASSERT_EQ(-ENOENT, r);
152+
ASSERT_TRUE(s.empty());
153+
ASSERT_FALSE(p);
154+
155+
r = rgw::run_coro(&dp, pool, maybethrowvs(0, instr,
156+
std::make_unique<int>(5)),
157+
std::tie(s, p), nullptr);
158+
ASSERT_EQ(0, r);
159+
ASSERT_EQ(instr, s);
160+
ASSERT_TRUE(p);
161+
ASSERT_EQ(5, *p);
162+
163+
s.clear();
164+
p.reset();
165+
166+
asio::spawn(pool,
167+
[&](asio::yield_context y) -> void {
168+
r = rgw::run_coro(&dp, pool,
169+
maybethrowvs(ENOENT, instr,
170+
std::make_unique<int>(5)),
171+
std::tie(s, p), "yielding", y);
172+
},
173+
async::use_blocked);
174+
ASSERT_EQ(-ENOENT, r);
175+
ASSERT_TRUE(s.empty());
176+
ASSERT_FALSE(p);
177+
178+
asio::spawn(pool,
179+
[&](asio::yield_context y) -> void {
180+
r = rgw::run_coro(&dp, pool,
181+
maybethrowvs(0, instr,
182+
std::make_unique<int>(5)),
183+
std::tie(s, p), "yielding", y);
184+
},
185+
async::use_blocked);
186+
ASSERT_EQ(0, r);
187+
ASSERT_EQ(instr, s);
188+
ASSERT_TRUE(p);
189+
ASSERT_EQ(5, *p);
190+
191+
s.clear();
192+
p.reset();
193+
r = rgw::run_coro(&dp, pool, maybethrowvs(ENOENT, instr,
194+
std::make_unique<int>(5)),
195+
std::tie(s, p), "blocking", null_yield);
196+
ASSERT_EQ(-ENOENT, r);
197+
ASSERT_TRUE(s.empty());
198+
ASSERT_FALSE(p);
199+
200+
r = rgw::run_coro(&dp, pool, maybethrowvs(0, instr,
201+
std::make_unique<int>(5)),
202+
std::tie(s, p), "blocking", null_yield);
203+
ASSERT_EQ(0, r);
204+
ASSERT_EQ(instr, s);
205+
ASSERT_TRUE(p);
206+
ASSERT_EQ(5, *p);
207+
}

0 commit comments

Comments
 (0)