rgw: RGWCoroutine::call(nullptr) sets retcode=0

callers generally expect 'yield call(...);' to set retcode with the
result of the given coroutine. it's valid to call() a nullptr, but
that's not always clear at the call site - for example, this critical
piece of code in RGWBucketSyncSingleEntryCR::operate():

      if (sync_status == 0) {
        /* update marker */
        set_status() << "calling marker_tracker->finish(" << entry_marker << ")";
        yield call(marker_tracker->finish(entry_marker));
        sync_status = retcode;
      }

marker_tracker->finish() only returns a coroutine when it needs to flush
the marker; otherwise it returns a nullptr. as a result, 'retcode' may
not be assigned as expected by the call(), and a previous value is
mistakenly returned up the stack

Fixes: https://tracker.ceph.com/issues/41412
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 7d12a11a14c4a9c8e814b907d83dc07783e11e80)

diff --git a/src/rgw/rgw_coroutine.cc b/src/rgw/rgw_coroutine.cc
index 4ef9a6ac94e2233a7b2eea0a0dd602efbd9779d4..1ccefc2daba54b827400e63ce0e0f5d951159a6f 100644 (file)
--- a/src/rgw/rgw_coroutine.cc
+++ b/src/rgw/rgw_coroutine.cc
@@ -874,7 +874,12 @@ void RGWCoroutinesManagerRegistry::dump(Formatter *f) const {
 
 void RGWCoroutine::call(RGWCoroutine *op)
 {
-  stack->call(op);
+  if (op) {
+    stack->call(op);
+  } else {
+    // the call()er expects this to set a retcode
+    retcode = 0;
+  }
 }
 
 RGWCoroutinesStack *RGWCoroutine::spawn(RGWCoroutine *op, bool wait)