summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbrain <brain@e03df62e-2008-0410-955e-edbf42e46eb7>2008-11-22 16:54:32 +0000
committerbrain <brain@e03df62e-2008-0410-955e-edbf42e46eb7>2008-11-22 16:54:32 +0000
commit9bddcf91ac79f34f8721bbf161f90cc4cc9dbe8c (patch)
tree7fb6284977dbd4f94e5e34b08f0a7311ed854075
parentf7844096dcbe912557b46b0a52b35cf7cf6fc07e (diff)
Thread safety fixes to avoid crashes on rehash, dont reopen logs within the rehash thread. Put this in the safe part of the rehash operation, after the thread exits. Put a mutex around the part where the thread exits, just in case somehow there are two rehash threads exiting at the same time due to user
muppetry. git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@10811 e03df62e-2008-0410-955e-edbf42e46eb7
-rw-r--r--include/inspircd.h8
-rw-r--r--src/configreader.cpp9
-rw-r--r--src/inspircd.cpp31
-rw-r--r--src/server.cpp2
4 files changed, 30 insertions, 20 deletions
diff --git a/include/inspircd.h b/include/inspircd.h
index 3993142c7..85c2f62e6 100644
--- a/include/inspircd.h
+++ b/include/inspircd.h
@@ -390,6 +390,8 @@ class CoreExport InspIRCd : public classbase
*/
std::map<BufferedSocket*,BufferedSocket*> SocketCull;
+ Mutex* RehashFinishMutex;
+
/** Globally accessible fake user record. This is used to force mode changes etc across s2s, etc.. bit ugly, but.. better than how this was done in 1.1
* Reason for it:
* kludge alert!
@@ -398,7 +400,7 @@ class CoreExport InspIRCd : public classbase
* hash and set its descriptor to FD_MAGIC_NUMBER so the data
* falls into the abyss :p
*/
- User *FakeClient;
+ User* FakeClient;
/** Returns the next available UID for this server.
*/
@@ -408,13 +410,13 @@ class CoreExport InspIRCd : public classbase
* @param nick The nickname to find
* @return A pointer to the user, or NULL if the user does not exist
*/
- User *FindUUID(const std::string &);
+ User* FindUUID(const std::string &);
/** Find a user in the UUID hash
* @param nick The nickname to find
* @return A pointer to the user, or NULL if the user does not exist
*/
- User *FindUUID(const char *);
+ User* FindUUID(const char *);
/** Build the ISUPPORT string by triggering all modules On005Numeric events
*/
diff --git a/src/configreader.cpp b/src/configreader.cpp
index 3be7d5945..9978f24d7 100644
--- a/src/configreader.cpp
+++ b/src/configreader.cpp
@@ -1227,12 +1227,6 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
// write once here, to try it out and make sure its ok
ServerInstance->WritePID(this->PID);
- /* Switch over logfiles */
- ServerInstance->Logs->CloseLogs();
- ServerInstance->Logs->OpenFileLogs();
-
- ServerInstance->Logs->Log("CONFIG", DEFAULT, "Done reading configuration file.");
-
/* If we're rehashing, let's load any new modules, and unload old ones
*/
if (!bail)
@@ -1323,9 +1317,6 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
}
}
}
-
- ServerInstance->Logs->Log("CONFIG", DEFAULT, "Successfully unloaded %lu of %lu modules and loaded %lu of %lu modules.",(unsigned long)rem,(unsigned long)removed_modules.size(),(unsigned long)add,(unsigned long)added_modules.size());
-
ServerInstance->Threads->Unlock();
}
diff --git a/src/inspircd.cpp b/src/inspircd.cpp
index 1b173e56e..4d112b314 100644
--- a/src/inspircd.cpp
+++ b/src/inspircd.cpp
@@ -220,6 +220,8 @@ void InspIRCd::Cleanup()
delete this->Logs;
this->Logs = 0;
}
+
+ delete RehashFinishMutex;
}
void InspIRCd::Restart(const std::string &reason)
@@ -631,6 +633,8 @@ InspIRCd::InspIRCd(int argc, char** argv)
ConfigThread->Run();
delete ConfigThread;
this->ConfigThread = NULL;
+ /* Switch over logfiles */
+ Logs->OpenFileLogs();
/** Note: This is safe, the method checks for user == NULL */
this->Parser->SetupCommandTable();
@@ -806,6 +810,8 @@ int InspIRCd::Run()
Exit(0);
}
+ RehashFinishMutex = Mutexes->CreateMutex();
+
while (true)
{
#ifndef WIN32
@@ -817,21 +823,22 @@ int InspIRCd::Run()
#endif
/* Check if there is a config thread which has finished executing but has not yet been freed */
+ RehashFinishMutex->Lock();
if (this->ConfigThread && this->ConfigThread->GetExitFlag())
{
/* Rehash has completed */
- this->Logs->Log("CONFIG",DEBUG,"Detected ConfigThread exiting, tidying up...");
- /* IMPORTANT: This delete may hang if you fuck up your thread syncronization.
- * It will hang waiting for the ConfigThread to 'join' to avoid race conditons,
- * until the other thread is completed.
- */
- delete ConfigThread;
- ConfigThread = NULL;
+ /* Switch over logfiles */
+ Logs->CloseLogs();
+ Logs->OpenFileLogs();
+
+ this->Logs->Log("CONFIG",DEBUG,"Detected ConfigThread exiting, tidying up...");
/* These are currently not known to be threadsafe, so they are executed outside
* of the thread. It would be pretty simple to move them to the thread Run method
- * once they are known threadsafe with all the correct mutexes in place.
+ * once they are known threadsafe with all the correct mutexes in place. This might
+ * not be worth the effort however as these functions execute relatively quickly
+ * and would not benefit from being within the config read thread.
*
* XXX: The order of these is IMPORTANT, do not reorder them without testing
* thoroughly!!!
@@ -844,7 +851,15 @@ int InspIRCd::Run()
User* user = !Config->RehashUserUID.empty() ? FindNick(Config->RehashUserUID) : NULL;
FOREACH_MOD_I(this, I_OnRehash, OnRehash(user, Config->RehashParameter));
this->BuildISupport();
+
+ /* IMPORTANT: This delete may hang if you fuck up your thread syncronization.
+ * It will hang waiting for the ConfigThread to 'join' to avoid race conditons,
+ * until the other thread is completed.
+ */
+ delete ConfigThread;
+ ConfigThread = NULL;
}
+ RehashFinishMutex->Unlock();
/* time() seems to be a pretty expensive syscall, so avoid calling it too much.
* Once per loop iteration is pleanty.
diff --git a/src/server.cpp b/src/server.cpp
index c3efc7e6b..44f2ce78f 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -48,6 +48,7 @@ void InspIRCd::Exit(int status)
void RehashHandler::Call(const std::string &reason)
{
+ Server->RehashFinishMutex->Lock();
Server->SNO->WriteToSnoMask('A', "Rehashing config file %s %s",ServerConfig::CleanFilename(Server->ConfigFileName), reason.c_str());
Server->RehashUsersAndChans();
FOREACH_MOD_I(Server, I_OnGarbageCollect, OnGarbageCollect());
@@ -59,6 +60,7 @@ void RehashHandler::Call(const std::string &reason)
Server->ConfigThread = new ConfigReaderThread(Server, false, "");
Server->Threads->Create(Server->ConfigThread);
}
+ Server->RehashFinishMutex->Unlock();
}
void InspIRCd::RehashServer()