Lock all Database operations.

All DB functions that are called outside of Database.cpp are now locked
by the same mutex. This might be a bit overkill, but it's not a hot code
path, so it doesn't matter. Better to avoid the potential deadlocks if
we made it too granular.

From now on a clear distinction must be made between external and
internal functions in Database.cpp, or else deadlock will occur.

Note that sqlite database operations are already locked, but if execute
multiple transactions within the same operation, it could have still
caused problems.

I also removed the DbPlayer fetch when writing to DB by making it a part
of the Player struct. This, by itself, should have fixed the crash we found.
This commit is contained in:
dongresource 2020-09-29 16:47:39 +02:00
parent 3f35d2e960
commit fb281b0237
2 changed files with 35 additions and 1 deletions

View File

@ -10,8 +10,16 @@
#include "contrib/sqlite/sqlite_orm.h" #include "contrib/sqlite/sqlite_orm.h"
#include "MissionManager.hpp" #include "MissionManager.hpp"
#if defined(__MINGW32__) && !defined(_GLIBCXX_HAS_GTHREADS)
#include "mingw/mingw.mutex.h"
#else
#include <mutex>
#endif
using namespace sqlite_orm; using namespace sqlite_orm;
std::mutex dbCrit;
# pragma region DatabaseScheme # pragma region DatabaseScheme
auto db = make_storage("database.db", auto db = make_storage("database.db",
make_table("Accounts", make_table("Accounts",
@ -123,6 +131,8 @@ int Database::getPlayersCount() {
int Database::addAccount(std::string login, std::string password) int Database::addAccount(std::string login, std::string password)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
password = BCrypt::generateHash(password); password = BCrypt::generateHash(password);
Account account = {}; Account account = {};
account.Login = login; account.Login = login;
@ -134,6 +144,8 @@ int Database::addAccount(std::string login, std::string password)
void Database::updateSelected(int accountId, int slot) void Database::updateSelected(int accountId, int slot)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
Account acc = db.get<Account>(accountId); Account acc = db.get<Account>(accountId);
acc.Selected = slot; acc.Selected = slot;
//timestamp //timestamp
@ -143,6 +155,8 @@ void Database::updateSelected(int accountId, int slot)
std::unique_ptr<Database::Account> Database::findAccount(std::string login) std::unique_ptr<Database::Account> Database::findAccount(std::string login)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
// this is awful, I've tried everything to improve it // this is awful, I've tried everything to improve it
auto find = db.get_all<Account>( auto find = db.get_all<Account>(
where(c(&Account::Login) == login), limit(1)); where(c(&Account::Login) == login), limit(1));
@ -154,6 +168,8 @@ std::unique_ptr<Database::Account> Database::findAccount(std::string login)
bool Database::isNameFree(sP_CL2LS_REQ_CHECK_CHAR_NAME* nameCheck) bool Database::isNameFree(sP_CL2LS_REQ_CHECK_CHAR_NAME* nameCheck)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
std::string First = U16toU8(nameCheck->szFirstName); std::string First = U16toU8(nameCheck->szFirstName);
std::string Last = U16toU8(nameCheck->szLastName); std::string Last = U16toU8(nameCheck->szLastName);
return return
@ -165,6 +181,8 @@ bool Database::isNameFree(sP_CL2LS_REQ_CHECK_CHAR_NAME* nameCheck)
int Database::createCharacter(sP_CL2LS_REQ_SAVE_CHAR_NAME* save, int AccountID) int Database::createCharacter(sP_CL2LS_REQ_SAVE_CHAR_NAME* save, int AccountID)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
// fail if the player already has 4 or more characters // fail if the player already has 4 or more characters
if (db.count<DbPlayer>(where(c(&DbPlayer::AccountID) == AccountID)) >= 4) if (db.count<DbPlayer>(where(c(&DbPlayer::AccountID) == AccountID)) >= 4)
return -1; return -1;
@ -216,6 +234,8 @@ int Database::createCharacter(sP_CL2LS_REQ_SAVE_CHAR_NAME* save, int AccountID)
void Database::finishCharacter(sP_CL2LS_REQ_CHAR_CREATE* character) void Database::finishCharacter(sP_CL2LS_REQ_CHAR_CREATE* character)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
DbPlayer finish = getDbPlayerById(character->PCStyle.iPC_UID); DbPlayer finish = getDbPlayerById(character->PCStyle.iPC_UID);
finish.AppearanceFlag = 1; finish.AppearanceFlag = 1;
finish.Body = character->PCStyle.iBody; finish.Body = character->PCStyle.iBody;
@ -256,6 +276,8 @@ void Database::finishCharacter(sP_CL2LS_REQ_CHAR_CREATE* character)
void Database::finishTutorial(int PlayerID) void Database::finishTutorial(int PlayerID)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
Player finish = getPlayer(PlayerID); Player finish = getPlayer(PlayerID);
// set flag // set flag
finish.PCStyle2.iTutorialFlag= 1; finish.PCStyle2.iTutorialFlag= 1;
@ -284,6 +306,8 @@ void Database::finishTutorial(int PlayerID)
int Database::deleteCharacter(int characterID, int userID) int Database::deleteCharacter(int characterID, int userID)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
auto find = auto find =
db.get_all<DbPlayer>(where(c(&DbPlayer::PlayerID) == characterID and c(&DbPlayer::AccountID)==userID)); db.get_all<DbPlayer>(where(c(&DbPlayer::PlayerID) == characterID and c(&DbPlayer::AccountID)==userID));
int slot = find.front().slot; int slot = find.front().slot;
@ -296,6 +320,8 @@ int Database::deleteCharacter(int characterID, int userID)
std::vector <Player> Database::getCharacters(int UserID) std::vector <Player> Database::getCharacters(int UserID)
{ {
std::lock_guard<std::mutex> lock(dbCrit);
std::vector<DbPlayer>characters = std::vector<DbPlayer>characters =
db.get_all<DbPlayer>(where db.get_all<DbPlayer>(where
(c(&DbPlayer::AccountID) == UserID)); (c(&DbPlayer::AccountID) == UserID));
@ -312,12 +338,16 @@ std::vector <Player> Database::getCharacters(int UserID)
// XXX: This is never called? // XXX: This is never called?
void Database::evaluateCustomName(int characterID, CustomName decision) { void Database::evaluateCustomName(int characterID, CustomName decision) {
std::lock_guard<std::mutex> lock(dbCrit);
DbPlayer player = getDbPlayerById(characterID); DbPlayer player = getDbPlayerById(characterID);
player.NameCheck = (int)decision; player.NameCheck = (int)decision;
db.update(player); db.update(player);
} }
void Database::changeName(sP_CL2LS_REQ_CHANGE_CHAR_NAME* save) { void Database::changeName(sP_CL2LS_REQ_CHANGE_CHAR_NAME* save) {
std::lock_guard<std::mutex> lock(dbCrit);
DbPlayer Player = getDbPlayerById(save->iPCUID); DbPlayer Player = getDbPlayerById(save->iPCUID);
Player.FirstName = U16toU8(save->szFirstName); Player.FirstName = U16toU8(save->szFirstName);
Player.LastName = U16toU8(save->szLastName); Player.LastName = U16toU8(save->szLastName);
@ -381,7 +411,7 @@ Database::DbPlayer Database::playerToDb(Player *player)
} }
//timestamp //timestamp
result.LastLogin = getTimestamp(); result.LastLogin = getTimestamp();
result.Created = getDbPlayerById(player->iID).Created; result.Created = player->creationTime;
return result; return result;
} }
@ -391,6 +421,7 @@ Player Database::DbToPlayer(DbPlayer player) {
result.iID = player.PlayerID; result.iID = player.PlayerID;
result.accountId = player.AccountID; result.accountId = player.AccountID;
result.creationTime = player.Created;
result.PCStyle2.iAppearanceFlag = player.AppearanceFlag; result.PCStyle2.iAppearanceFlag = player.AppearanceFlag;
result.PCStyle.iBody = player.Body; result.PCStyle.iBody = player.Body;
result.PCStyle.iClass = player.Class; result.PCStyle.iClass = player.Class;
@ -468,6 +499,8 @@ Player Database::getPlayer(int id) {
#pragma region ShardServer #pragma region ShardServer
void Database::updatePlayer(Player *player) { void Database::updatePlayer(Player *player) {
std::lock_guard<std::mutex> lock(dbCrit);
DbPlayer toUpdate = playerToDb(player); DbPlayer toUpdate = playerToDb(player);
db.update(toUpdate); db.update(toUpdate);
updateInventory(player); updateInventory(player);

View File

@ -16,6 +16,7 @@ struct Player {
int64_t SerialKey; int64_t SerialKey;
int32_t iID; int32_t iID;
uint64_t FEKey; uint64_t FEKey;
time_t creationTime;
int level; int level;
int HP; int HP;