Don't delete users immediately when quit or killed, instead wait until message processing is done

include/anope.h

@@ -158,15 +158,28 @@ namespace Anope
 		/**
 		 * Trim leading and trailing white spaces from the string.
 		 */
-		inline string& trim()
+
+		inline string& ltrim()
 		{
 			while (!this->_string.empty() && isspace(this->_string[0]))
 				this->_string.erase(this->_string.begin());
+			return *this;
+		}
+
+		inline string& rtrim()
+		{
 			while (!this->_string.empty() && isspace(this->_string[this->_string.length() - 1]))
 				this->_string.erase(this->_string.length() - 1);
 			return *this;
 		}
 
+		inline string& trim()
+		{
+			this->ltrim();
+			this->rtrim();
+			return *this;
+		}
+
 		/**
 		 * Clears the string.
 		 */

include/commands.h

@@ -54,7 +54,7 @@ class CoreExport CommandSource
 	/* The nick executing the command */
 	Anope::string nick;
 	/* User executing the command, may be NULL */
-	Reference<User> u;
+	User *u;
  public:
 	/* The account executing the command */
 	Reference<NickCore> nc;

include/modules.h

@@ -291,7 +291,7 @@ class CoreExport Module : public Extensible
 	 * @param u The connecting user.
 	 * @param exempt set to true/is true if the user should be excepted from bans etc
 	 */
-	virtual void OnUserConnect(Reference<User> &u, bool &exempt) { }
+	virtual void OnUserConnect(User *u, bool &exempt) { }
 
 	/** Called when a new server connects to the network.
 	 * @param s The server that has connected to the network

include/sockets.h

@@ -261,10 +261,10 @@ class CoreExport Socket : public Flags<SocketFlag>
 class CoreExport BufferedSocket : public virtual Socket
 {
  protected:
+ 	/* Things read from the socket */
+ 	Anope::string read_buffer;
 	/* Things to be written to the socket */
 	Anope::string write_buffer;
-	/* Part of a message sent from the server, but not totally received */
-	Anope::string extra_buf;
 	/* How much data was received from this socket on this recv() */
 	int recv_len;
 
@@ -282,11 +282,9 @@ class CoreExport BufferedSocket : public virtual Socket
 	 */
 	bool ProcessWrite() anope_override;
 
-	/** Called with a line received from the socket
-	 * @param buf The line
-	 * @return true to continue reading, false to drop the socket
+	/** Gets the new line from the input buffer, if any
 	 */
-	virtual bool Read(const Anope::string &buf);
+	const Anope::string GetLine();
 
 	/** Write to the socket
 	* @param message The message

include/uplink.h

@@ -26,9 +26,9 @@ class UplinkSocket : public ConnectionSocket, public BufferedSocket
  public:
 	UplinkSocket();
 	~UplinkSocket();
-	bool Read(const Anope::string &);
-	void OnConnect();
-	void OnError(const Anope::string &);
+	bool ProcessRead() anope_override;
+	void OnConnect() anope_override;
+	void OnError(const Anope::string &) anope_override;
 
 	/* A message sent over the uplink socket */
 	class CoreExport Message

include/users.h

@@ -32,6 +32,10 @@ extern CoreExport time_t MaxUserTime;
 /* Online user and channel data. */
 class CoreExport User : public virtual Base, public Extensible, public CommandReply
 {
+	/* true if the user was quit or killed */
+	bool quit;
+	/* Users that are in the process of quitting */
+	static std::list<User *> quitting_users;
  protected:
 	Anope::string vident;
 	Anope::string ident;
@@ -103,10 +107,12 @@ class CoreExport User : public virtual Base, public Extensible, public CommandRe
 	 */
 	User(const Anope::string &snick, const Anope::string &sident, const Anope::string &shost, const Anope::string &svhost, const Anope::string &sip, Server *sserver, const Anope::string &srealname, time_t ssignon, const Anope::string &smodes, const Anope::string &suid = "");
 
+ protected:
 	/** Destroy a user.
 	 */
 	virtual ~User();
 
+ public:
 	/** Update the nickname of a user record accordingly, should be
 	 * called from ircd protocol.
 	 * @param newnick The new username
@@ -327,6 +333,13 @@ class CoreExport User : public virtual Base, public Extensible, public CommandRe
 	 */
 	void KillInternal(const Anope::string &source, const Anope::string &reason);
 
+	/** Processes a quit for the user, and marks them as quit
+	 * @param reason The reason for the quit
+	 */
+	void Quit(const Anope::string &reason = "");
+
+	bool Quitting() const;
+
 	/* Returns a mask that will most likely match any address the
 	 * user will have from that location.  For IP addresses, wildcards the
 	 * appropriate subnet mask (e.g. 35.1.1.1 -> 35.*; 128.2.1.1 -> 128.2.*);
@@ -348,6 +361,10 @@ class CoreExport User : public virtual Base, public Extensible, public CommandRe
 	 * @return the user, if they exist
 	 */
 	static User* Find(const Anope::string &name, bool nick_only = false);
+
+	/** Quits all users who are pending to be quit
+	 */
+	static void QuitUsers();
 };
 
 #endif // USERS_H

modules/commands/cs_seen.cpp

@@ -351,9 +351,9 @@ class CSSeen : public Module
 			purger.SetSecs(expiretimeout);
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (u)
+		if (!u->Quitting())
 			UpdateUser(u, NEW, u->nick, "", "", "");
 	}
 

modules/commands/os_defcon.cpp

@@ -408,27 +408,6 @@ class OSDefcon : public Module
 		this->ParseModeString();
 	}
 
-	EventReturn OnUserConnect(User *u, bool &exempt)
-	{
-		if (!exempt && u->server->IsSynced() && DConfig.Check(DEFCON_AKILL_NEW_CLIENTS) && !u->server->IsULined())
-		{
-			if (DConfig.Check(DEFCON_AKILL_NEW_CLIENTS) && akills)
-			{
-				Log(OperServ, "operserv/defcon") << "DEFCON: adding akill for *@" << u->host;
-				XLine *x = new XLine("*@" + u->host, Config->OperServ, Anope::CurTime + DConfig.akillexpire, DConfig.akillreason, XLineManager::GenerateUID());
-				x->by = Config->OperServ;
-				akills->AddXLine(x);
-			}
-
-			if (DConfig.Check(DEFCON_NO_NEW_CLIENTS) || DConfig.Check(DEFCON_AKILL_NEW_CLIENTS))
-				u->Kill(Config->OperServ, DConfig.akillreason);
-
-			return EVENT_STOP;
-		}
-
-		return EVENT_CONTINUE;
-	}
-
 	EventReturn OnChannelModeSet(Channel *c, MessageSource &, ChannelModeName Name, const Anope::string &param) anope_override
 	{
 		ChannelMode *cm = ModeManager::FindChannelModeByName(Name);
@@ -501,9 +480,9 @@ class OSDefcon : public Module
 		return EVENT_CONTINUE;
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (exempt || !u || !u->server->IsSynced() || u->server->IsULined())
+		if (exempt || !u->Quitting() || !u->server->IsSynced() || u->server->IsULined())
 			return;
 
 		if (DConfig.Check(DEFCON_AKILL_NEW_CLIENTS) && akills)

modules/commands/os_dns.cpp

@@ -719,9 +719,9 @@ class ModuleDNS : public Module
 		}
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (u && u->server)
+		if (!u->Quitting() && u->server)
 		{
 			DNSServer *s = DNSServer::Find(u->server->GetName());
 			/* Check for user limit reached */

modules/commands/os_forbid.cpp

@@ -252,9 +252,9 @@ class OSForbid : public Module
 		ModuleManager::Attach(i, this, sizeof(i) / sizeof(Implementation));
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (!u || exempt)
+		if (u->Quitting() || exempt)
 			return;
 
 		this->OnUserNickChange(u, "");

modules/commands/os_news.cpp

@@ -405,9 +405,9 @@ class OSNews : public Module
 			DisplayNews(u, NEWS_OPER);
 	}
 
-	void OnUserConnect(Reference<User> &user, bool &) anope_override
+	void OnUserConnect(User *user, bool &) anope_override
 	{
-		if (!user || !user->server->IsSynced())
+		if (user->Quitting() || !user->server->IsSynced())
 			return;
 
 		DisplayNews(user, NEWS_LOGON);

modules/commands/os_session.cpp

@@ -718,9 +718,9 @@ class OSSession : public Module
 		ModuleManager::SetPriority(this, PRIORITY_FIRST);
 	}
 
-	void OnUserConnect(Reference<User> &user, bool &exempt) anope_override
+	void OnUserConnect(User *user, bool &exempt) anope_override
 	{
-		if (user && Config->LimitSessions)
+		if (!user->Quitting() && Config->LimitSessions)
 			this->AddSession(user, exempt);
 	}
 

modules/extra/m_dnsbl.cpp

@@ -129,9 +129,9 @@ class ModuleDNSBL : public Module
 		}
 	}
 
-	void OnUserConnect(Reference<User> &user, bool &exempt) anope_override
+	void OnUserConnect(User *user, bool &exempt) anope_override
 	{
-		if (exempt || !user || (!this->check_on_connect && !Me->IsSynced()) || !dnsmanager)
+		if (exempt || user->Quitting() || (!this->check_on_connect && !Me->IsSynced()) || !dnsmanager)
 			return;
 
 		if (!this->check_on_netburst && !user->server->IsSynced())

modules/extra/m_mysql.cpp

@@ -406,7 +406,7 @@ std::vector<Query> MySQLService::CreateTable(const Anope::string &table, const D
 	return queries;
 }
 
-Query MySQLService::BuildInsert(const Anope::string &table, unsigned int id, Data &data) anope_override
+Query MySQLService::BuildInsert(const Anope::string &table, unsigned int id, Data &data)
 {
 	/* Empty columns not present in the data set */
 	const std::set<Anope::string> &known_cols = this->active_schema[table];

modules/extra/m_proxyscan.cpp

@@ -125,9 +125,10 @@ class HTTPProxyConnect : public ProxyConnect, public BufferedSocket
 		return "HTTP";
 	}
 
-	bool Read(const Anope::string &buf) anope_override
+	bool ProcessRead() anope_override
 	{
-		if (buf == ProxyCheckString)
+		BufferedSocket::ProcessRead();
+		if (this->GetLine() == ProxyCheckString)
 		{
 			this->Ban();
 			return false;
@@ -341,9 +342,9 @@ class ModuleProxyScan : public Module
 		}
 	}
 
-	void OnUserConnect(Reference<User> &user, bool &exempt) anope_override
+	void OnUserConnect(User *user, bool &exempt) anope_override
 	{
-		if (exempt || !user || !Me->IsSynced() || !user->server->IsSynced())
+		if (exempt || user->Quitting() || !Me->IsSynced() || !user->server->IsSynced())
 			return;
 
 		/* At this time we only support IPv4 */

modules/pseudoclients/botserv.cpp

@@ -150,14 +150,11 @@ class BotServCore : public Module
 		if (MOD_RESULT == EVENT_STOP)
 			return;
 
-		Reference<User> user_reference(u);
 		Reference<NickCore> nc_reference(u->Account());
 		cmd->Execute(source, params);
-
-		if (user_reference && nc_reference)
-		{
-			FOREACH_MOD(I_OnPostCommand, OnPostCommand(source, cmd, params));
-		}
+		if (!nc_reference)
+			source.nc = NULL;
+		FOREACH_MOD(I_OnPostCommand, OnPostCommand(source, cmd, params));
 	}
 
 	void OnJoinChannel(User *user, Channel *c) anope_override

modules/pseudoclients/nickserv.cpp

@@ -303,9 +303,9 @@ class NickServCore : public Module
 		}
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (!u || !u->server->IsSynced())
+		if (u->Quitting() || !u->server->IsSynced())
 			return;
 
 		const NickAlias *na = NickAlias::Find(u->nick);

modules/pseudoclients/operserv.cpp

@@ -237,9 +237,9 @@ class OperServCore : public Module
 			Log(u, "oper", OperServ) << "is no longer an IRC operator";
 	}
 
-	void OnUserConnect(Reference<User> &u, bool &exempt) anope_override
+	void OnUserConnect(User *u, bool &exempt) anope_override
 	{
-		if (u && !exempt)
+		if (!u->Quitting() && !exempt)
 			XLineManager::CheckAll(u);
 	}
 
@@ -249,11 +249,9 @@ class OperServCore : public Module
 			this->sqlines.CheckAllXLines(u);
 	}
 
-	EventReturn OnCheckKick(User *u, ChannelInfo *ci, bool &kick) anope_override
+	EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) anope_override
 	{
-		if (this->sqlines.CheckChannel(ci->c))
-			kick = true;
-		return EVENT_CONTINUE;
+		return this->sqlines.CheckChannel(ci->c) ? EVENT_STOP : EVENT_CONTINUE;
 	}
 
 	EventReturn OnPreHelp(CommandSource &source, const std::vector<Anope::string> &params) anope_override

src/command.cpp

@@ -262,13 +262,10 @@ void RunCommand(CommandSource &source, const Anope::string &message)
 		return;
 	}
 
-	bool had_u = source.GetUser(), had_nc = source.nc;
-	Reference<User> user_reference(source.GetUser());
 	Reference<NickCore> nc_reference(source.nc);
 	c->Execute(source, params);
-	if (had_u == user_reference && had_nc == nc_reference)
-	{
-		FOREACH_MOD(I_OnPostCommand, OnPostCommand(source, c, params));
-	}
+	if (!nc_reference)
+		source.nc = NULL;
+	FOREACH_MOD(I_OnPostCommand, OnPostCommand(source, c, params));
 }
 

src/messages.cpp

@@ -351,7 +351,7 @@ void Quit::Run(MessageSource &source, const std::vector<Anope::string> &params)
 		na->last_quit = reason;
 	}
 	FOREACH_MOD(I_OnUserQuit, OnUserQuit(user, reason));
-	delete user;
+	user->Quit(reason);
 
 	return;
 }

src/nickalias.cpp

@@ -88,7 +88,7 @@ void NickAlias::Release()
 			User *u = User::Find(this->nick);
 			if (u && u->server == Me)
 			{
-				delete u;
+				u->Quit();
 			}
 		}
 

src/servers.cpp

@@ -137,7 +137,8 @@ Server::~Server()
 					na->last_quit = this->quit_reason;
 				}
 
-				delete u;
+				u->Quit(this->quit_reason);
+				u->server = NULL;
 			}
 		}
 

src/socket_transport.cpp

@@ -33,34 +33,9 @@ bool BufferedSocket::ProcessRead()
 		return false;
 	
 	tbuffer[len] = 0;
+	this->read_buffer.append(tbuffer);
 	this->recv_len = len;
 
-	Anope::string sbuffer = this->extra_buf;
-	sbuffer += tbuffer;
-	this->extra_buf.clear();
-	size_t lastnewline = sbuffer.rfind('\n');
-	if (lastnewline == Anope::string::npos)
-	{
-		this->extra_buf = sbuffer;
-		return true;
-	}
-	if (lastnewline < sbuffer.length() - 1)
-	{
-		this->extra_buf = sbuffer.substr(lastnewline);
-		this->extra_buf.trim();
-		sbuffer = sbuffer.substr(0, lastnewline);
-	}
-
-	sepstream stream(sbuffer, '\n');
-
-	Anope::string tbuf;
-	while (stream.GetToken(tbuf))
-	{
-		tbuf.trim();
-		if (!Read(tbuf))
-			return false;
-	}
-
 	return true;
 }
 
@@ -76,9 +51,15 @@ bool BufferedSocket::ProcessWrite()
 	return true;
 }
 
-bool BufferedSocket::Read(const Anope::string &buf)
+const Anope::string BufferedSocket::GetLine()
 {
-	return false;
+	size_t s = this->read_buffer.find('\n');
+	if (s == Anope::string::npos)
+		return "";
+	Anope::string str = this->read_buffer.substr(0, s + 1);
+	this->read_buffer.erase(0, s + 1);
+	this->read_buffer.ltrim();
+	return str.trim();
 }
 
 void BufferedSocket::Write(const char *buffer, size_t l)

src/uplink.cpp

@@ -116,9 +116,14 @@ UplinkSocket::~UplinkSocket()
 	}
 }
 
-bool UplinkSocket::Read(const Anope::string &buf)
+bool UplinkSocket::ProcessRead()
 {
-	Anope::Process(buf);
+	BufferedSocket::ProcessRead();
+	for (Anope::string buf; (buf = this->GetLine()).empty() == false;)
+	{
+		Anope::Process(buf);
+		User::QuitUsers();
+	}
 	return true;
 }
 

src/users.cpp

@@ -30,12 +30,15 @@ int OperCount = 0;
 unsigned MaxUserCount = 0;
 time_t MaxUserTime = 0;
 
+std::list<User *> User::quitting_users;
+
 User::User(const Anope::string &snick, const Anope::string &sident, const Anope::string &shost, const Anope::string &svhost, const Anope::string &sip, Server *sserver, const Anope::string &srealname, time_t ssignon, const Anope::string &smodes, const Anope::string &suid)
 {
 	if (snick.empty() || sident.empty() || shost.empty())
 		throw CoreException("Bad args passed to User::User");
 
 	/* we used to do this by calloc, no more. */
+	quit = false;
 	server = NULL;
 	invalid_pw_count = invalid_pw_time = lastmemosend = lastnickreg = lastmail = 0;
 	on_access = false;
@@ -79,8 +82,7 @@ User::User(const Anope::string &snick, const Anope::string &sident, const Anope:
 	bool exempt = false;
 	if (server && server->IsULined())
 		exempt = true;
-	Reference<User> user = this;
-	FOREACH_MOD(I_OnUserConnect, OnUserConnect(user, exempt));
+	FOREACH_MOD(I_OnUserConnect, OnUserConnect(this, exempt));
 }
 
 void User::ChangeNick(const Anope::string &newnick, time_t ts)
@@ -229,10 +231,11 @@ void User::SetRealname(const Anope::string &srealname)
 
 User::~User()
 {
-	Log(LOG_DEBUG_2) << "User::~User() called";
-
-	Log(this, "disconnect") << "(" << this->realname << ") " << "disconnected from the network (" << this->server->GetName() << ")";
-	--this->server->users;
+	if (this->server != NULL)
+	{
+		Log(this, "disconnect") << "(" << this->realname << ") disconnected from the network (" << this->server->GetName() << ")";
+		--this->server->users;
+	}
 
 	FOREACH_MOD(I_OnUserLogoff, OnUserLogoff(this));
 
@@ -252,8 +255,6 @@ User::~User()
 	NickAlias *na = NickAlias::Find(this->nick);
 	if (na)
 		na->OnCancel(this);
-
-	Log(LOG_DEBUG_2) << "User::~User() done";
 }
 
 void User::SendMessage(const BotInfo *source, const char *fmt, ...)
@@ -752,6 +753,12 @@ void User::Kill(const Anope::string &source, const Anope::string &reason)
 
 void User::KillInternal(const Anope::string &source, const Anope::string &reason)
 {
+	if (this->quit)
+	{
+		Log(LOG_DEBUG) << "Duplicate quit for " << this->nick;
+		return;
+	}
+
 	Log(this, "killed") << "was killed by " << source << " (Reason: " << reason << ")";
 
 	NickAlias *na = NickAlias::Find(this->nick);
@@ -761,7 +768,25 @@ void User::KillInternal(const Anope::string &source, const Anope::string &reason
 		na->last_quit = reason;
 	}
 
-	delete this;
+	this->quit = true;
+	quitting_users.push_back(this);
+}
+
+void User::Quit(const Anope::string &reason)
+{
+	if (this->quit)
+	{
+		Log(LOG_DEBUG) << "Duplicate quit for " << this->nick;
+		return;
+	}
+
+	this->quit = true;
+	quitting_users.push_back(this);
+}
+
+bool User::Quitting() const
+{
+	return this->quit;
 }
 
 Anope::string User::Mask() const
@@ -829,3 +854,10 @@ User* User::Find(const Anope::string &name, bool nick_only)
 	return NULL;
 }
 
+void User::QuitUsers()
+{
+	for (std::list<User *>::iterator it = quitting_users.begin(), it_end = quitting_users.end(); it != it_end; ++it)
+		delete *it;
+	quitting_users.clear();
+}
+