Skip to content

Commit

Permalink
rgw/dbstore: Fix crash in delete_stale_objs
Browse files Browse the repository at this point in the history 
Fix a race between RemoveBucket and delete_stale_objs operations
by using shared_ptr to add reference to DB Ops.

Fixes:https://tracker.ceph.com/issues/55828
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
  • Loading branch information
@soumyakoduri
soumyakoduri committed Jun 23, 2022
1 parent 61967a4 commit fc09dad
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 91 deletions.
8 changes: 4 additions & 4 deletions src/rgw/store/dbstore/common/dbstore.cc
View file
Expand Up @@ -91,7 +91,7 @@ int DB::Destroy(const DoutPrefixProvider *dpp)
}


DBOp *DB::getDBOp(const DoutPrefixProvider *dpp, std::string_view Op,
std::shared_ptr<class DBOp> DB::getDBOp(const DoutPrefixProvider *dpp, std::string_view Op,
const DBOpParams *params)
{
if (!Op.compare("InsertUser"))
Expand Down Expand Up @@ -138,7 +138,7 @@ DBOp *DB::getDBOp(const DoutPrefixProvider *dpp, std::string_view Op,
ldpp_dout(dpp, 30)<<"No objectmap found for bucket: " \
<<params->op.bucket.info.bucket.name << dendl;
/* not found */
return NULL;
return nullptr;
}

Ob = iter->second;
Expand All @@ -164,7 +164,7 @@ DBOp *DB::getDBOp(const DoutPrefixProvider *dpp, std::string_view Op,
if (!Op.compare("DeleteStaleObjectData"))
return Ob->DeleteStaleObjectData;

return NULL;
return nullptr;
}

int DB::objectmapInsert(const DoutPrefixProvider *dpp, string bucket, class ObjectOp* ptr)
Expand Down Expand Up @@ -243,7 +243,7 @@ int DB::InitializeParams(const DoutPrefixProvider *dpp, DBOpParams *params)

int DB::ProcessOp(const DoutPrefixProvider *dpp, std::string_view Op, DBOpParams *params) {
int ret = -1;
class DBOp *db_op;
shared_ptr<class DBOp> db_op;

db_op = getDBOp(dpp, Op, params);

Expand Down
52 changes: 26 additions & 26 deletions src/rgw/store/dbstore/common/dbstore.h
View file
Expand Up @@ -341,21 +341,21 @@ struct DBOpPrepareParams {
};

struct DBOps {
class InsertUserOp *InsertUser;
class RemoveUserOp *RemoveUser;
class GetUserOp *GetUser;
class InsertBucketOp *InsertBucket;
class UpdateBucketOp *UpdateBucket;
class RemoveBucketOp *RemoveBucket;
class GetBucketOp *GetBucket;
class ListUserBucketsOp *ListUserBuckets;
class InsertLCEntryOp *InsertLCEntry;
class RemoveLCEntryOp *RemoveLCEntry;
class GetLCEntryOp *GetLCEntry;
class ListLCEntriesOp *ListLCEntries;
class InsertLCHeadOp *InsertLCHead;
class RemoveLCHeadOp *RemoveLCHead;
class GetLCHeadOp *GetLCHead;
std::shared_ptr<class InsertUserOp> InsertUser;
std::shared_ptr<class RemoveUserOp> RemoveUser;
std::shared_ptr<class GetUserOp> GetUser;
std::shared_ptr<class InsertBucketOp> InsertBucket;
std::shared_ptr<class UpdateBucketOp> UpdateBucket;
std::shared_ptr<class RemoveBucketOp> RemoveBucket;
std::shared_ptr<class GetBucketOp> GetBucket;
std::shared_ptr<class ListUserBucketsOp> ListUserBuckets;
std::shared_ptr<class InsertLCEntryOp> InsertLCEntry;
std::shared_ptr<class RemoveLCEntryOp> RemoveLCEntry;
std::shared_ptr<class GetLCEntryOp> GetLCEntry;
std::shared_ptr<class ListLCEntriesOp> ListLCEntries;
std::shared_ptr<class InsertLCHeadOp> InsertLCHead;
std::shared_ptr<class RemoveLCHeadOp> RemoveLCHead;
std::shared_ptr<class GetLCHeadOp> GetLCHead;
};

class ObjectOp {
Expand All @@ -364,16 +364,16 @@ class ObjectOp {

virtual ~ObjectOp() {}

class PutObjectOp *PutObject;
class DeleteObjectOp *DeleteObject;
class GetObjectOp *GetObject;
class UpdateObjectOp *UpdateObject;
class ListBucketObjectsOp *ListBucketObjects;
class PutObjectDataOp *PutObjectData;
class UpdateObjectDataOp *UpdateObjectData;
class GetObjectDataOp *GetObjectData;
class DeleteObjectDataOp *DeleteObjectData;
class DeleteStaleObjectDataOp *DeleteStaleObjectData;
std::shared_ptr<class PutObjectOp> PutObject;
std::shared_ptr<class DeleteObjectOp> DeleteObject;
std::shared_ptr<class GetObjectOp> GetObject;
std::shared_ptr<class UpdateObjectOp> UpdateObject;
std::shared_ptr<class ListBucketObjectsOp> ListBucketObjects;
std::shared_ptr<class PutObjectDataOp> PutObjectData;
std::shared_ptr<class UpdateObjectDataOp> UpdateObjectData;
std::shared_ptr<class GetObjectDataOp> GetObjectData;
std::shared_ptr<class DeleteObjectDataOp> DeleteObjectData;
std::shared_ptr<class DeleteStaleObjectDataOp> DeleteStaleObjectData;

virtual int InitializeObjectOps(std::string db_name, const DoutPrefixProvider *dpp) { return 0; }
virtual int FreeObjectOps(const DoutPrefixProvider *dpp) { return 0; }
Expand Down Expand Up @@ -1512,7 +1512,7 @@ class DB {

int InitializeParams(const DoutPrefixProvider *dpp, DBOpParams *params);
int ProcessOp(const DoutPrefixProvider *dpp, std::string_view Op, DBOpParams *params);
DBOp* getDBOp(const DoutPrefixProvider *dpp, std::string_view Op, const DBOpParams *params);
std::shared_ptr<class DBOp> getDBOp(const DoutPrefixProvider *dpp, std::string_view Op, const DBOpParams *params);
int objectmapInsert(const DoutPrefixProvider *dpp, std::string bucket, class ObjectOp* ptr);
int objectmapDelete(const DoutPrefixProvider *dpp, std::string bucket);

Expand Down
86 changes: 25 additions & 61 deletions src/rgw/store/dbstore/sqlite/sqliteDB.cc
View file
Expand Up @@ -591,42 +591,21 @@ static int list_lc_head(const DoutPrefixProvider *dpp, DBOpInfo &op, sqlite3_stm
int SQLiteDB::InitializeDBOps(const DoutPrefixProvider *dpp)
{
(void)createTables(dpp);
dbops.InsertUser = new SQLInsertUser(&this->db, this->getDBname(), cct);
dbops.RemoveUser = new SQLRemoveUser(&this->db, this->getDBname(), cct);
dbops.GetUser = new SQLGetUser(&this->db, this->getDBname(), cct);
dbops.InsertBucket = new SQLInsertBucket(&this->db, this->getDBname(), cct);
dbops.UpdateBucket = new SQLUpdateBucket(&this->db, this->getDBname(), cct);
dbops.RemoveBucket = new SQLRemoveBucket(&this->db, this->getDBname(), cct);
dbops.GetBucket = new SQLGetBucket(&this->db, this->getDBname(), cct);
dbops.ListUserBuckets = new SQLListUserBuckets(&this->db, this->getDBname(), cct);
dbops.InsertLCEntry = new SQLInsertLCEntry(&this->db, this->getDBname(), cct);
dbops.RemoveLCEntry = new SQLRemoveLCEntry(&this->db, this->getDBname(), cct);
dbops.GetLCEntry = new SQLGetLCEntry(&this->db, this->getDBname(), cct);
dbops.ListLCEntries = new SQLListLCEntries(&this->db, this->getDBname(), cct);
dbops.InsertLCHead = new SQLInsertLCHead(&this->db, this->getDBname(), cct);
dbops.RemoveLCHead = new SQLRemoveLCHead(&this->db, this->getDBname(), cct);
dbops.GetLCHead = new SQLGetLCHead(&this->db, this->getDBname(), cct);

return 0;
}

int SQLiteDB::FreeDBOps(const DoutPrefixProvider *dpp)
{
delete dbops.InsertUser;
delete dbops.RemoveUser;
delete dbops.GetUser;
delete dbops.InsertBucket;
delete dbops.UpdateBucket;
delete dbops.RemoveBucket;
delete dbops.GetBucket;
delete dbops.ListUserBuckets;
delete dbops.InsertLCEntry;
delete dbops.RemoveLCEntry;
delete dbops.GetLCEntry;
delete dbops.ListLCEntries;
delete dbops.InsertLCHead;
delete dbops.RemoveLCHead;
delete dbops.GetLCHead;
dbops.InsertUser = make_shared<SQLInsertUser>(&this->db, this->getDBname(), cct);
dbops.RemoveUser = make_shared<SQLRemoveUser>(&this->db, this->getDBname(), cct);
dbops.GetUser = make_shared<SQLGetUser>(&this->db, this->getDBname(), cct);
dbops.InsertBucket = make_shared<SQLInsertBucket>(&this->db, this->getDBname(), cct);
dbops.UpdateBucket = make_shared<SQLUpdateBucket>(&this->db, this->getDBname(), cct);
dbops.RemoveBucket = make_shared<SQLRemoveBucket>(&this->db, this->getDBname(), cct);
dbops.GetBucket = make_shared<SQLGetBucket>(&this->db, this->getDBname(), cct);
dbops.ListUserBuckets = make_shared<SQLListUserBuckets>(&this->db, this->getDBname(), cct);
dbops.InsertLCEntry = make_shared<SQLInsertLCEntry>(&this->db, this->getDBname(), cct);
dbops.RemoveLCEntry = make_shared<SQLRemoveLCEntry>(&this->db, this->getDBname(), cct);
dbops.GetLCEntry = make_shared<SQLGetLCEntry>(&this->db, this->getDBname(), cct);
dbops.ListLCEntries = make_shared<SQLListLCEntries>(&this->db, this->getDBname(), cct);
dbops.InsertLCHead = make_shared<SQLInsertLCHead>(&this->db, this->getDBname(), cct);
dbops.RemoveLCHead = make_shared<SQLRemoveLCHead>(&this->db, this->getDBname(), cct);
dbops.GetLCHead = make_shared<SQLGetLCHead>(&this->db, this->getDBname(), cct);

return 0;
}
Expand Down Expand Up @@ -1062,31 +1041,16 @@ int SQLiteDB::ListAllObjects(const DoutPrefixProvider *dpp, DBOpParams *params)

int SQLObjectOp::InitializeObjectOps(string db_name, const DoutPrefixProvider *dpp)
{
PutObject = new SQLPutObject(sdb, db_name, cct);
DeleteObject = new SQLDeleteObject(sdb, db_name, cct);
GetObject = new SQLGetObject(sdb, db_name, cct);
UpdateObject = new SQLUpdateObject(sdb, db_name, cct);
ListBucketObjects = new SQLListBucketObjects(sdb, db_name, cct);
PutObjectData = new SQLPutObjectData(sdb, db_name, cct);
UpdateObjectData = new SQLUpdateObjectData(sdb, db_name, cct);
GetObjectData = new SQLGetObjectData(sdb, db_name, cct);
DeleteObjectData = new SQLDeleteObjectData(sdb, db_name, cct);
DeleteStaleObjectData = new SQLDeleteStaleObjectData(sdb, db_name, cct);

return 0;
}

int SQLObjectOp::FreeObjectOps(const DoutPrefixProvider *dpp)
{
delete PutObject;
delete DeleteObject;
delete GetObject;
delete UpdateObject;
delete PutObjectData;
delete UpdateObjectData;
delete GetObjectData;
delete DeleteObjectData;
delete DeleteStaleObjectData;
PutObject = make_shared<SQLPutObject>(sdb, db_name, cct);
DeleteObject = make_shared<SQLDeleteObject>(sdb, db_name, cct);
GetObject = make_shared<SQLGetObject>(sdb, db_name, cct);
UpdateObject = make_shared<SQLUpdateObject>(sdb, db_name, cct);
ListBucketObjects = make_shared<SQLListBucketObjects>(sdb, db_name, cct);
PutObjectData = make_shared<SQLPutObjectData>(sdb, db_name, cct);
UpdateObjectData = make_shared<SQLUpdateObjectData>(sdb, db_name, cct);
GetObjectData = make_shared<SQLGetObjectData>(sdb, db_name, cct);
DeleteObjectData = make_shared<SQLDeleteObjectData>(sdb, db_name, cct);
DeleteStaleObjectData = make_shared<SQLDeleteStaleObjectData>(sdb, db_name, cct);

return 0;
}
Expand Down

0 comments on commit fc09dad

Please sign in to comment.