From f3275bfc229219b8ce44c054c8a5f9fc34567fac Mon Sep 17 00:00:00 2001 From: einhorn_b Date: Wed, 16 Jun 2021 15:19:20 +0200 Subject: [PATCH] changes for hoepfully fixing Session deadlock problems --- .../src/cpp/JSONInterface/JsonGetLogin.cpp | 4 +- .../JsonRequestHandlerFactory.cpp | 2 +- .../cpp/SingletonManager/SessionManager.cpp | 67 +++++++++---------- login_server/src/cpp/model/Session.cpp | 36 +++++----- .../src/cpp/model/table/ModelBase.cpp | 20 ++---- 5 files changed, 55 insertions(+), 74 deletions(-) diff --git a/login_server/src/cpp/JSONInterface/JsonGetLogin.cpp b/login_server/src/cpp/JSONInterface/JsonGetLogin.cpp index 92ef5e887..76f4e6524 100644 --- a/login_server/src/cpp/JSONInterface/JsonGetLogin.cpp +++ b/login_server/src/cpp/JSONInterface/JsonGetLogin.cpp @@ -10,8 +10,6 @@ Poco::JSON::Object* JsonGetLogin::handle(Poco::Dynamic::Var params) { - - int session_id = 0; auto sm = SessionManager::getInstance(); auto pt = PendingTasksManager::getInstance(); auto observer = SingletonTaskObserver::getInstance(); @@ -58,4 +56,4 @@ Poco::JSON::Object* JsonGetLogin::handle(Poco::Dynamic::Var params) //printf("[JsonGetLogin] %s\n", user_string.data()); return result; -} \ No newline at end of file +} diff --git a/login_server/src/cpp/JSONInterface/JsonRequestHandlerFactory.cpp b/login_server/src/cpp/JSONInterface/JsonRequestHandlerFactory.cpp index 19772f3e1..b50fc7957 100644 --- a/login_server/src/cpp/JSONInterface/JsonRequestHandlerFactory.cpp +++ b/login_server/src/cpp/JSONInterface/JsonRequestHandlerFactory.cpp @@ -65,7 +65,7 @@ Poco::Net::HTTPRequestHandler* JsonRequestHandlerFactory::createRequestHandler(c auto sm = SessionManager::getInstance(); Session* s = nullptr; - if (!session_id) { + if (session_id) { s = sm->getSession(session_id); } diff --git a/login_server/src/cpp/SingletonManager/SessionManager.cpp b/login_server/src/cpp/SingletonManager/SessionManager.cpp index a26ecb0ab..114cff0c9 100644 --- a/login_server/src/cpp/SingletonManager/SessionManager.cpp +++ b/login_server/src/cpp/SingletonManager/SessionManager.cpp @@ -57,17 +57,17 @@ bool SessionManager::init() //case VALIDATE_ONLY_URL: mValidations[i] = new Poco::RegularExpression("^https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}$"); break; case VALIDATE_ONLY_URL: mValidations[i] = new Poco::RegularExpression("^https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\/?"); break; case VALIDATE_HAS_SPECIAL_CHARACTER: mValidations[i] = new Poco::RegularExpression(".*[@$!%*?&+-].*"); break; - case VALIDATE_HAS_UPPERCASE_LETTER: - mValidations[i] = new Poco::RegularExpression(".*[A-Z].*"); + case VALIDATE_HAS_UPPERCASE_LETTER: + mValidations[i] = new Poco::RegularExpression(".*[A-Z].*"); ServerConfig::g_ServerKeySeed->put(i, DRRandom::r64()); break; case VALIDATE_HAS_LOWERCASE_LETTER: mValidations[i] = new Poco::RegularExpression(".*[a-z].*"); break; default: printf("[SessionManager::%s] unknown validation type\n", __FUNCTION__); } } - - mInitalized = true; + + mInitalized = true; mWorkingMutex.unlock(); return true; } @@ -98,7 +98,7 @@ void SessionManager::deinitalize() } printf("[SessionManager::deinitalize] count of dead locked sessions: %d\n", mDeadLockedSessionCount); - + mInitalized = false; mWorkingMutex.unlock(); } @@ -142,7 +142,7 @@ Session* SessionManager::getNewSession(int* handle) // first check if we have any timeouted session to directly reuse it checkTimeoutSession(); - // lock + // lock try { //Poco::Mutex::ScopedLock _lock(mWorkingMutex, 500); mWorkingMutex.tryLock(500); @@ -156,8 +156,8 @@ Session* SessionManager::getNewSession(int* handle) //UniLib::controller::TaskPtr checkSessionTimeout(new CheckSessionTimeouted); //checkSessionTimeout->scheduleTask(checkSessionTimeout); - // check if we have an existing session ready to use - while (mEmptyRequestStack.size() > 0) { + // check if we have an existing session ready to use + while (mEmptyRequestStack.size() > 0) { int local_handle = mEmptyRequestStack.top(); mEmptyRequestStack.pop(); auto resultIt = mRequestSessionMap.find(local_handle); @@ -186,10 +186,10 @@ Session* SessionManager::getNewSession(int* handle) mRequestSessionMap.erase(local_handle); } - + } } - + // else create new RequestSession Object // calculate random handle // check if already exist, if get new @@ -211,7 +211,7 @@ Session* SessionManager::getNewSession(int* handle) //printf("[SessionManager::getNewSession] handle: %ld, sum: %u\n", newHandle, mRequestSessionMap.size()); mWorkingMutex.unlock(); return requestSession; - + //return nullptr; } @@ -231,7 +231,7 @@ bool SessionManager::releaseSession(int requestHandleSession) return false; } //mWorkingMutex.lock(); - + auto it = mRequestSessionMap.find(requestHandleSession); if (it == mRequestSessionMap.end()) { //printf("[SessionManager::releaseRequestSession] requestSession with handle: %d not found\n", requestHandleSession); @@ -242,16 +242,15 @@ bool SessionManager::releaseSession(int requestHandleSession) // delete session, not reuse as workaround for server freeze bug - mRequestSessionMap.erase(requestHandleSession); + /*mRequestSessionMap.erase(requestHandleSession); delete session; mWorkingMutex.unlock(); return true; - +*/ // check if dead locked - if (session->tryLock()) { - session->unlock(); + if (!session->isDeadLocked()) { session->reset(); session->setActive(false); } @@ -264,9 +263,9 @@ bool SessionManager::releaseSession(int requestHandleSession) mWorkingMutex.unlock(); return true; } - + // change request handle we don't want session hijacking - + // hardcoded disabled session max if (mEmptyRequestStack.size() > 100) { mRequestSessionMap.erase(requestHandleSession); @@ -285,11 +284,11 @@ bool SessionManager::releaseSession(int requestHandleSession) mWorkingMutex.unlock(); return true; } - + session->setHandle(newHandle); mRequestSessionMap.insert(std::pair(newHandle, session)); mEmptyRequestStack.push(newHandle); - + mWorkingMutex.unlock(); return true; } @@ -354,13 +353,11 @@ Session* SessionManager::getSession(int handle) } if (0 == handle) return nullptr; Session* result = nullptr; - try { - //Poco::Mutex::ScopedLock _lock(mWorkingMutex, 500); - mWorkingMutex.tryLock(500); - } - catch (Poco::TimeoutException &ex) { - printf("[SessionManager::getSession] exception timout mutex: %s\n", ex.displayText().data()); - return result; + + + if(!mWorkingMutex.tryLock(500)) { + printf("[SessionManager::getSession] exception timout mutex: \n"); + return result; } //mWorkingMutex.lock(); auto it = mRequestSessionMap.find(handle); @@ -376,14 +373,12 @@ Session* SessionManager::getSession(int handle) return nullptr; } if (0 == iResult) { - //printf("[SessionManager::getSession] session isn't active\n"); mWorkingMutex.unlock(); return nullptr; } //result->setActive(true); result->updateTimeout(); } - //printf("[SessionManager::getSession] handle: %ld\n", handle); mWorkingMutex.unlock(); return result; } @@ -418,8 +413,8 @@ Session* SessionManager::findByUserId(int userId) } //mWorkingMutex.lock(); for (auto it = mRequestSessionMap.begin(); it != mRequestSessionMap.end(); it++) { - while (it->second->isDeadLocked()) - { + while (it->second->isDeadLocked()) + { it = mRequestSessionMap.erase(it); mDeadLockedSessionCount++; auto em = ErrorManager::getInstance(); @@ -484,7 +479,7 @@ std::vector SessionManager::findAllByUserId(int userId) Session* SessionManager::findByEmail(const std::string& email) { assert(email.size() > 0); - + try { //Poco::Mutex::ScopedLock _lock(mWorkingMutex, 500); mWorkingMutex.tryLock(500); @@ -605,23 +600,23 @@ bool SessionManager::checkPwdValidation(const std::string& pwd, NotificationList if (!isValid(pwd, VALIDATE_PASSWORD)) { errorReciver->addError(new Error( - lang->gettext("Password"), + lang->gettext("Password"), lang->gettext("Please enter a valid password with at least 8 characters, upper and lower case letters, at least one number and one special character (@$!%*?&+-_)!"))); // @$!%*?&+- if (pwd.size() < 8) { errorReciver->addError(new Error( - lang->gettext("Password"), + lang->gettext("Password"), lang->gettext("Your password is to short!"))); } else if (!isValid(pwd, VALIDATE_HAS_LOWERCASE_LETTER)) { errorReciver->addError(new Error( - lang->gettext("Password"), + lang->gettext("Password"), lang->gettext("Your password does not contain lowercase letters!"))); } else if (!isValid(pwd, VALIDATE_HAS_UPPERCASE_LETTER)) { errorReciver->addError(new Error( - lang->gettext("Password"), + lang->gettext("Password"), lang->gettext("Your password does not contain any capital letters!"))); } else if (!isValid(pwd, VALIDATE_HAS_NUMBER)) { diff --git a/login_server/src/cpp/model/Session.cpp b/login_server/src/cpp/model/Session.cpp index 767f1acb3..42b7202fd 100644 --- a/login_server/src/cpp/model/Session.cpp +++ b/login_server/src/cpp/model/Session.cpp @@ -84,29 +84,30 @@ void Session::reset() int Session::isActive() { int ret = 0; - try { - mWorkMutex.tryLock(100); - } - catch (Poco::TimeoutException &ex) { - return -1; - } + + if(!mWorkMutex.tryLock(100)) { + return -1; + } + ret = (int)mActive; - unlock(); + + try { + unlock(); + } catch(Poco::SystemException& ex) { + addError(new ParamError("Session::isActive", "exception unlocking mutex", ex.what())); + return -1; + } return ret; } bool Session::isDeadLocked() { - try { - mWorkMutex.tryLock(200); - unlock(); - return false; - } - catch (Poco::Exception& ex) { - - } - return true; + if(!mWorkMutex.tryLock(200)) { + return true; + }; + unlock(); + return false; } bool Session::setActive(bool active) @@ -922,12 +923,11 @@ bool Session::useOrGeneratePassphrase(const std::string& passphase) bool Session::lastTransactionTheSame(Poco::AutoPtr newTransaction) { assert(!newTransaction.isNull()); - lock(); + Poco::ScopedLock _lock(mWorkMutex); if (mLastTransaction.isNull()) { return false; } bool result = mLastTransaction->isTheSameTransaction(newTransaction); - unlock(); return result; } diff --git a/login_server/src/cpp/model/table/ModelBase.cpp b/login_server/src/cpp/model/table/ModelBase.cpp index b8f68c369..737db3f98 100644 --- a/login_server/src/cpp/model/table/ModelBase.cpp +++ b/login_server/src/cpp/model/table/ModelBase.cpp @@ -124,14 +124,8 @@ namespace model { void ModelBase::duplicate() { - //Poco::ScopedLock _lock(mWorkMutex); - std::string stack_details = "[ModelBase::duplicate] table: "; - stack_details += getTableName(); - lock(stack_details.data()); + Poco::ScopedLock _lock(mWorkMutex); mReferenceCount++; - printf("[ModelBase::duplicate] new value: %d, table name: %s\n", mReferenceCount, getTableName()); - unlock(); - //printf("[ModelBase::duplicate] new value: %d\n", mReferenceCount); } void ModelBase::release() @@ -139,21 +133,15 @@ namespace model { if(mReferenceCount <= 0) { throw Poco::Exception("ModelBase already released", getTableName()); } - std::string stack_details = "[ModelBase::release] table: "; - stack_details += getTableName(); - stack_details += ", reference count: "; - stack_details += std::to_string(mReferenceCount); - lock(stack_details.data()); + + Poco::ScopedLock _lock(mWorkMutex); mReferenceCount--; - printf("[ModelBase::release] new value: %d, table name: %s\n", mReferenceCount, getTableName()); + if (0 == mReferenceCount) { - unlock(); delete this; return; } - unlock(); - } Poco::Data::Statement ModelBase::_loadFromDB(Poco::Data::Session session, const std::vector& fieldNames, MysqlConditionType conditionType/* = MYSQL_CONDITION_AND*/)