Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw/dbstore: Fix crash in delete_stale_objs #46829

Merged
merged 1 commit into from Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 4 additions & 10 deletions src/rgw/store/dbstore/common/dbstore.cc
Expand Up @@ -82,16 +82,14 @@ int DB::Destroy(const DoutPrefixProvider *dpp)
closeDB(dpp);


FreeDBOps(dpp);

ldpp_dout(dpp, 20)<<"DB successfully destroyed - name:" \
<<db_name << dendl;

return 0;
}


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 +136,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 +162,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 @@ -198,7 +196,6 @@ int DB::objectmapInsert(const DoutPrefixProvider *dpp, string bucket, class Obje
int DB::objectmapDelete(const DoutPrefixProvider *dpp, string bucket)
{
map<string, class ObjectOp*>::iterator iter;
class ObjectOp *Ob;

const std::lock_guard<std::mutex> lk(mtx);
iter = DB::objectmap.find(bucket);
Expand All @@ -212,9 +209,6 @@ int DB::objectmapDelete(const DoutPrefixProvider *dpp, string bucket)
return 0;
}

Ob = (class ObjectOp*) (iter->second);
Ob->FreeObjectOps(dpp);

DB::objectmap.erase(iter);

return 0;
Expand Down Expand Up @@ -243,7 +237,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
54 changes: 26 additions & 28 deletions src/rgw/store/dbstore/common/dbstore.h
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,19 +364,18 @@ 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; }
};

class DBOp {
Expand Down Expand Up @@ -1512,7 +1511,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 All @@ -1521,7 +1520,6 @@ class DB {
virtual int closeDB(const DoutPrefixProvider *dpp) { return 0; }
virtual int createTables(const DoutPrefixProvider *dpp) { return 0; }
virtual int InitializeDBOps(const DoutPrefixProvider *dpp) { return 0; }
virtual int FreeDBOps(const DoutPrefixProvider *dpp) { return 0; }
virtual int InitPrepareParams(const DoutPrefixProvider *dpp,
DBOpPrepareParams &p_params,
DBOpParams* params) = 0;
Expand Down
86 changes: 25 additions & 61 deletions src/rgw/store/dbstore/sqlite/sqliteDB.cc
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
2 changes: 0 additions & 2 deletions src/rgw/store/dbstore/sqlite/sqliteDB.h
Expand Up @@ -34,7 +34,6 @@ class SQLiteDB : public DB, virtual public DBOp {
void *openDB(const DoutPrefixProvider *dpp) override;
int closeDB(const DoutPrefixProvider *dpp) override;
int InitializeDBOps(const DoutPrefixProvider *dpp) override;
int FreeDBOps(const DoutPrefixProvider *dpp) override;

int InitPrepareParams(const DoutPrefixProvider *dpp, DBOpPrepareParams &p_params,
DBOpParams* params) override;
Expand Down Expand Up @@ -82,7 +81,6 @@ class SQLObjectOp : public ObjectOp {
~SQLObjectOp() {}

int InitializeObjectOps(std::string db_name, const DoutPrefixProvider *dpp);
int FreeObjectOps(const DoutPrefixProvider *dpp);
};

class SQLInsertUser : public SQLiteDB, public InsertUserOp {
Expand Down