rgw: don't pass object context into async coroutines

Fixes: http://tracker.ceph.com/issues/15625
The async read data/attrs coroutines may outlive the callers. Instead
of introducing a complicated refcounting scheme, just don't pass it in.
Anyway, it was not thread safe, and the benefit of using it is not
clear.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
(cherry picked from commit b225d1758057efc30721aad25447d54fa36e4cb2)

diff --git a/src/rgw/rgw_cr_rados.h b/src/rgw/rgw_cr_rados.h
index 2459e78732638baa0ac94430b5e37054c18f0fa7..1f00931ba75d7b798aff3768493e63c85d583ef7 100644 (file)
--- a/src/rgw/rgw_cr_rados.h
+++ b/src/rgw/rgw_cr_rados.h
@@ -174,7 +174,7 @@ template <class T>
 class RGWSimpleRadosReadCR : public RGWSimpleCoroutine {
   RGWAsyncRadosProcessor *async_rados;
   RGWRados *store;
-  RGWObjectCtx& obj_ctx;
+  RGWObjectCtx obj_ctx;
   bufferlist bl;
 
   rgw_bucket pool;
@@ -188,11 +188,10 @@ class RGWSimpleRadosReadCR : public RGWSimpleCoroutine {
 
 public:
   RGWSimpleRadosReadCR(RGWAsyncRadosProcessor *_async_rados, RGWRados *_store,
-                     RGWObjectCtx& _obj_ctx,
                      const rgw_bucket& _pool, const string& _oid,
                      T *_result) : RGWSimpleCoroutine(_store->ctx()),
                                                 async_rados(_async_rados), store(_store),
-                                                obj_ctx(_obj_ctx),
+                                                obj_ctx(store),
                                                pool(_pool), oid(_oid),
                                                 pattrs(NULL),
                                                result(_result),
@@ -252,7 +251,7 @@ int RGWSimpleRadosReadCR<T>::request_complete()
 class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine {
   RGWAsyncRadosProcessor *async_rados;
   RGWRados *store;
-  RGWObjectCtx& obj_ctx;
+  RGWObjectCtx obj_ctx;
   bufferlist bl;
 
   rgw_bucket pool;
@@ -264,11 +263,10 @@ class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine {
 
 public:
   RGWSimpleRadosReadAttrsCR(RGWAsyncRadosProcessor *_async_rados, RGWRados *_store,
-                     RGWObjectCtx& _obj_ctx,
                      rgw_bucket& _pool, const string& _oid,
                      map<string, bufferlist> *_pattrs) : RGWSimpleCoroutine(_store->ctx()),
                                                 async_rados(_async_rados), store(_store),
-                                                obj_ctx(_obj_ctx),
+                                                obj_ctx(store),
                                                pool(_pool), oid(_oid),
                                                 pattrs(_pattrs),
                                                 req(NULL) { }

diff --git a/src/rgw/rgw_data_sync.cc b/src/rgw/rgw_data_sync.cc
index 6d73b73549115cb56fef003f56e2be6823446677..71db08edeb44d044de0e9321aa024a91d6e81048 100644 (file)
--- a/src/rgw/rgw_data_sync.cc
+++ b/src/rgw/rgw_data_sync.cc
@@ -46,18 +46,15 @@ void rgw_datalog_shard_data::decode_json(JSONObj *obj) {
 class RGWReadDataSyncStatusCoroutine : public RGWSimpleRadosReadCR<rgw_data_sync_info> {
   RGWDataSyncEnv *sync_env;
 
-  RGWObjectCtx& obj_ctx;
-
   rgw_data_sync_status *sync_status;
 
 public:
-  RGWReadDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env, RGWObjectCtx& _obj_ctx,
-                     rgw_data_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store, _obj_ctx,
+  RGWReadDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env,
+                     rgw_data_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store,
                                                                            _sync_env->store->get_zone_params().log_pool,
                                                                            RGWDataSyncStatusManager::sync_status_oid(_sync_env->source_zone),
                                                                            &_status->sync_info),
                                                                             sync_env(_sync_env),
-                                                                            obj_ctx(_obj_ctx),
                                                                            sync_status(_status) {}
 
   int handle_data(rgw_data_sync_info& data);
@@ -72,7 +69,7 @@ int RGWReadDataSyncStatusCoroutine::handle_data(rgw_data_sync_info& data)
   map<uint32_t, rgw_data_sync_marker>& markers = sync_status->sync_markers;
   RGWRados *store = sync_env->store;
   for (int i = 0; i < (int)data.num_shards; i++) {
-    spawn(new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, obj_ctx, store->get_zone_params().log_pool,
+    spawn(new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, store->get_zone_params().log_pool,
                                                     RGWDataSyncStatusManager::shard_obj_name(sync_env->source_zone, i), &markers[i]), true);
   }
   return 0;
@@ -344,7 +341,6 @@ class RGWInitDataSyncStatusCoroutine : public RGWCoroutine {
   RGWDataSyncEnv *sync_env;
 
   RGWRados *store;
-  RGWObjectCtx& obj_ctx;
 
   string sync_status_oid;
 
@@ -354,9 +350,8 @@ class RGWInitDataSyncStatusCoroutine : public RGWCoroutine {
   map<int, RGWDataChangesLogInfo> shards_info;
 public:
   RGWInitDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env,
-                     RGWObjectCtx& _obj_ctx, uint32_t _num_shards) : RGWCoroutine(_sync_env->cct),
-                                                sync_env(_sync_env), store(sync_env->store),
-                                                obj_ctx(_obj_ctx) {
+                     uint32_t _num_shards) : RGWCoroutine(_sync_env->cct),
+                                                sync_env(_sync_env), store(sync_env->store) {
     lock_name = "sync_lock";
     status.num_shards = _num_shards;
 
@@ -527,8 +522,7 @@ int RGWRemoteDataLog::get_shard_info(int shard_id)
 
 int RGWRemoteDataLog::read_sync_status(rgw_data_sync_status *sync_status)
 {
-  RGWObjectCtx obj_ctx(store, NULL);
-  int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, obj_ctx, sync_status));
+  int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, sync_status));
   if (r == -ENOENT) {
     r = 0;
   }
@@ -537,8 +531,7 @@ int RGWRemoteDataLog::read_sync_status(rgw_data_sync_status *sync_status)
 
 int RGWRemoteDataLog::init_sync_status(int num_shards)
 {
-  RGWObjectCtx obj_ctx(store, NULL);
-  return run(new RGWInitDataSyncStatusCoroutine(&sync_env, obj_ctx, num_shards));
+  return run(new RGWInitDataSyncStatusCoroutine(&sync_env, num_shards));
 }
 
 static string full_data_sync_index_shard_oid(const string& source_zone, int shard_id)
@@ -1139,8 +1132,7 @@ public:
 
   RGWCoroutine *alloc_finisher_cr() {
     RGWRados *store = sync_env->store;
-    RGWObjectCtx obj_ctx(store, NULL);
-    return new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, obj_ctx, store->get_zone_params().log_pool,
+    return new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, store->get_zone_params().log_pool,
                                                     RGWDataSyncStatusManager::shard_obj_name(sync_env->source_zone, shard_id), &sync_marker);
   }
 
@@ -1160,8 +1152,6 @@ class RGWDataSyncCR : public RGWCoroutine {
   RGWDataSyncEnv *sync_env;
   uint32_t num_shards;
 
-  RGWObjectCtx obj_ctx;
-
   rgw_data_sync_status sync_status;
 
   RGWDataSyncShardMarkerTrack *marker_tracker;
@@ -1175,7 +1165,6 @@ public:
   RGWDataSyncCR(RGWDataSyncEnv *_sync_env, uint32_t _num_shards, bool *_reset_backoff) : RGWCoroutine(_sync_env->cct),
                                                       sync_env(_sync_env),
                                                       num_shards(_num_shards),
-                                                      obj_ctx(sync_env->store),
                                                       marker_tracker(NULL),
                                                       shard_crs_lock("RGWDataSyncCR::shard_crs_lock"),
                                                       reset_backoff(_reset_backoff) {
@@ -1191,7 +1180,7 @@ public:
     reenter(this) {
 
       /* read sync status */
-      yield call(new RGWReadDataSyncStatusCoroutine(sync_env, obj_ctx, &sync_status));
+      yield call(new RGWReadDataSyncStatusCoroutine(sync_env, &sync_status));
 
       if (retcode == -ENOENT) {
         sync_status.sync_info.num_shards = num_shards;
@@ -1203,7 +1192,7 @@ public:
       /* state: init status */
       if ((rgw_data_sync_info::SyncState)sync_status.sync_info.state == rgw_data_sync_info::StateInit) {
         ldout(sync_env->cct, 20) << __func__ << "(): init" << dendl;
-        yield call(new RGWInitDataSyncStatusCoroutine(sync_env, obj_ctx, sync_status.sync_info.num_shards));
+        yield call(new RGWInitDataSyncStatusCoroutine(sync_env, sync_status.sync_info.num_shards));
         if (retcode < 0) {
           ldout(sync_env->cct, 0) << "ERROR: failed to init sync, retcode=" << retcode << dendl;
           return set_cr_error(retcode);
@@ -1320,9 +1309,7 @@ void RGWRemoteDataLog::wakeup(int shard_id, set<string>& keys) {
 
 int RGWRemoteDataLog::run_sync(int num_shards, rgw_data_sync_status& sync_status)
 {
-  RGWObjectCtx obj_ctx(store, NULL);
-
-  int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, obj_ctx, &sync_status));
+  int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, &sync_status));
   if (r < 0 && r != -ENOENT) {
     ldout(store->ctx(), 0) << "ERROR: failed to read sync status from source_zone=" << sync_env.source_zone << " r=" << r << dendl;
     return r;
@@ -1604,7 +1591,6 @@ void rgw_bucket_shard_inc_sync_marker::encode_attr(map<string, bufferlist>& attr
 
 class RGWReadBucketSyncStatusCoroutine : public RGWCoroutine {
   RGWDataSyncEnv *sync_env;
-  RGWObjectCtx obj_ctx;
   string oid;
   rgw_bucket_shard_sync_info *status;
 
@@ -1614,7 +1600,6 @@ public:
                       const string& _bucket_name, const string _bucket_id, int _shard_id,
                      rgw_bucket_shard_sync_info *_status) : RGWCoroutine(_sync_env->cct),
                                                             sync_env(_sync_env),
-                                                            obj_ctx(sync_env->store),
                                                             oid(RGWBucketSyncStatusManager::status_oid(sync_env->source_zone, _bucket_name, _bucket_id, _shard_id)),
                                                             status(_status) {}
   int operate();
@@ -1623,7 +1608,7 @@ public:
 int RGWReadBucketSyncStatusCoroutine::operate()
 {
   reenter(this) {
-    yield call(new RGWSimpleRadosReadAttrsCR(sync_env->async_rados, sync_env->store, obj_ctx,
+    yield call(new RGWSimpleRadosReadAttrsCR(sync_env->async_rados, sync_env->store,
                                                    sync_env->store->get_zone_params().log_pool,
                                                    oid,
                                                    &attrs));

diff --git a/src/rgw/rgw_sync.cc b/src/rgw/rgw_sync.cc
index 922a9048a6c43ed1e483d6cad9b014b39511b272..dab98fce5910855df136079af8fd9b69c2b1f593 100644 (file)
--- a/src/rgw/rgw_sync.cc
+++ b/src/rgw/rgw_sync.cc
@@ -587,17 +587,15 @@ bool RGWListRemoteMDLogCR::spawn_next() {
 
 class RGWInitSyncStatusCoroutine : public RGWCoroutine {
   RGWMetaSyncEnv *sync_env;
-  RGWObjectCtx& obj_ctx;
 
   rgw_meta_sync_info status;
   vector<RGWMetadataLogInfo> shards_info;
   RGWContinuousLeaseCR *lease_cr;
 public:
   RGWInitSyncStatusCoroutine(RGWMetaSyncEnv *_sync_env,
-                             RGWObjectCtx& _obj_ctx,
                              const rgw_meta_sync_info &status)
     : RGWCoroutine(_sync_env->store->ctx()), sync_env(_sync_env),
-      obj_ctx(_obj_ctx), status(status), shards_info(status.num_shards),
+      status(status), shards_info(status.num_shards),
       lease_cr(NULL) {}
 
   ~RGWInitSyncStatusCoroutine() {
@@ -689,19 +687,16 @@ public:
 
 class RGWReadSyncStatusCoroutine : public RGWSimpleRadosReadCR<rgw_meta_sync_info> {
   RGWMetaSyncEnv *sync_env;
-  RGWObjectCtx& obj_ctx;
 
   rgw_meta_sync_status *sync_status;
 
 public:
   RGWReadSyncStatusCoroutine(RGWMetaSyncEnv *_sync_env,
-                     RGWObjectCtx& _obj_ctx,
-                     rgw_meta_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store, _obj_ctx,
+                     rgw_meta_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store,
                                                                            _sync_env->store->get_zone_params().log_pool,
                                                                            _sync_env->status_oid(),
                                                                            &_status->sync_info),
                                                                             sync_env(_sync_env),
-                                                                            obj_ctx(_obj_ctx),
                                                                            sync_status(_status) {
 
   }
@@ -718,7 +713,7 @@ int RGWReadSyncStatusCoroutine::handle_data(rgw_meta_sync_info& data)
   RGWRados *store = sync_env->store;
   map<uint32_t, rgw_meta_sync_marker>& markers = sync_status->sync_markers;
   for (int i = 0; i < (int)data.num_shards; i++) {
-    spawn(new RGWSimpleRadosReadCR<rgw_meta_sync_marker>(sync_env->async_rados, store, obj_ctx, store->get_zone_params().log_pool,
+    spawn(new RGWSimpleRadosReadCR<rgw_meta_sync_marker>(sync_env->async_rados, store, store->get_zone_params().log_pool,
                                                    sync_env->shard_obj_name(i), &markers[i]), true);
   }
   return 0;
@@ -1636,8 +1631,6 @@ class RGWMetaSyncShardControlCR : public RGWBackoffControlCR
   rgw_meta_sync_marker sync_marker;
   const std::string period_marker;
 
-  RGWObjectCtx obj_ctx;
-
 public:
   RGWMetaSyncShardControlCR(RGWMetaSyncEnv *_sync_env, const rgw_bucket& _pool,
                             const std::string& period, RGWMetadataLog* mdlog,
@@ -1645,8 +1638,7 @@ public:
                             std::string&& period_marker)
     : RGWBackoffControlCR(_sync_env->cct, true), sync_env(_sync_env),
       pool(_pool), period(period), mdlog(mdlog), shard_id(_shard_id),
-      sync_marker(_marker), period_marker(std::move(period_marker)),
-      obj_ctx(sync_env->store) {}
+      sync_marker(_marker), period_marker(std::move(period_marker)) {}
 
   RGWCoroutine *alloc_cr() {
     return new RGWMetaSyncShardCR(sync_env, pool, period, mdlog, shard_id,
@@ -1655,7 +1647,7 @@ public:
 
   RGWCoroutine *alloc_finisher_cr() {
     RGWRados *store = sync_env->store;
-    return new RGWSimpleRadosReadCR<rgw_meta_sync_marker>(sync_env->async_rados, store, obj_ctx, pool,
+    return new RGWSimpleRadosReadCR<rgw_meta_sync_marker>(sync_env->async_rados, store, pool,
                                                                sync_env->shard_obj_name(shard_id), &sync_marker);
   }
 };
@@ -1784,8 +1776,7 @@ int RGWRemoteMetaLog::read_sync_status()
     return 0;
   }
 
-  RGWObjectCtx obj_ctx(store, NULL);
-  return run(new RGWReadSyncStatusCoroutine(&sync_env, obj_ctx, &sync_status));
+  return run(new RGWReadSyncStatusCoroutine(&sync_env, &sync_status));
 }
 
 int RGWRemoteMetaLog::init_sync_status()
@@ -1810,8 +1801,7 @@ int RGWRemoteMetaLog::init_sync_status()
     }
   }
 
-  RGWObjectCtx obj_ctx(store, NULL);
-  return run(new RGWInitSyncStatusCoroutine(&sync_env, obj_ctx, sync_info));
+  return run(new RGWInitSyncStatusCoroutine(&sync_env, sync_info));
 }
 
 int RGWRemoteMetaLog::store_sync_info()
@@ -1867,7 +1857,6 @@ int RGWRemoteMetaLog::run_sync()
     return 0;
   }
 
-  RGWObjectCtx obj_ctx(store, NULL);
   int r = 0;
 
   // get shard count and oldest log period from master
@@ -1897,7 +1886,7 @@ int RGWRemoteMetaLog::run_sync()
       ldout(store->ctx(), 1) << __func__ << "(): going down" << dendl;
       return 0;
     }
-    r = run(new RGWReadSyncStatusCoroutine(&sync_env, obj_ctx, &sync_status));
+    r = run(new RGWReadSyncStatusCoroutine(&sync_env, &sync_status));
     if (r < 0 && r != -ENOENT) {
       ldout(store->ctx(), 0) << "ERROR: failed to fetch sync status r=" << r << dendl;
       return r;
@@ -1922,8 +1911,7 @@ int RGWRemoteMetaLog::run_sync()
         sync_status.sync_info.period = cursor.get_period().get_id();
         sync_status.sync_info.realm_epoch = cursor.get_epoch();
       }
-      r = run(new RGWInitSyncStatusCoroutine(&sync_env, obj_ctx,
-                                             sync_status.sync_info));
+      r = run(new RGWInitSyncStatusCoroutine(&sync_env, sync_status.sync_info));
       if (r == -EBUSY) {
         backoff.backoff_sleep();
         continue;
@@ -1944,7 +1932,7 @@ int RGWRemoteMetaLog::run_sync()
 
   RGWPeriodHistory::Cursor cursor;
   do {
-    r = run(new RGWReadSyncStatusCoroutine(&sync_env, obj_ctx, &sync_status));
+    r = run(new RGWReadSyncStatusCoroutine(&sync_env, &sync_status));
     if (r < 0 && r != -ENOENT) {
       ldout(store->ctx(), 0) << "ERROR: failed to fetch sync status r=" << r << dendl;
       return r;