Added OnUserDisconnect method to modules.* to fix fd leak in m_ident.cpp

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@1134 e03df62e-2008-0410-955e-edbf42e46eb7

6 files changed, 66 insertions, 36 deletions

diff --git a/include/modules.h b/include/modules.h
index fa62c00df..150796cfe 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -159,9 +159,18 @@ class Module : public classbase
 
 	/** Called when a user quits.
 	 * The details of the exiting user are available to you in the parameter userrec *user
+	 * This event is only called when the user is fully registered when they quit. To catch
+	 * raw disconnections, use the OnUserDisconnect method.
 	 */
 	virtual void OnUserQuit(userrec* user);
 
+	/** Called whenever a user's socket is closed.
+	 * The details of the exiting user are available to you in the parameter userrec *user
+	 * This event is called for all users, registered or not, as a cleanup method for modules
+	 * which might assign resources to user, such as dns lookups, objects and sockets.
+	 */
+	virtual void OnUserDisconnect(userrec* user);
+
 	/** Called when a user joins a channel.
 	 * The details of the joining user are available to you in the parameter userrec *user,
 	 * and the details of the channel they have joined is available in the variable chanrec *channel
diff --git a/src/commands.cpp b/src/commands.cpp
index 088dabe68..e69ad41f6 100644
--- a/src/commands.cpp
+++ b/src/commands.cpp
@@ -93,7 +93,6 @@ extern bool nofork;
 
 extern time_t TIME;
 
-extern std::vector<int> fd_reap;
 extern std::vector<std::string> module_names;
 
 extern char MyExecutable[1024];
@@ -927,8 +926,14 @@ void handle_quit(char **parameters, int pcnt, userrec *user)
 		AddWhoWas(user);
 	}
 
+	FOREACH_MOD OnUserDisconnect(user);
+
 	/* push the socket on a stack of sockets due to be closed at the next opportunity */
-	fd_reap.push_back(user->fd);
+	if (user->fd > -1)
+	{
+		shutdown(user->fd,2);
+		close(user->fd);
+	}
 	
 	if (iter != clientlist.end())
 	{
diff --git a/src/dns.cpp b/src/dns.cpp
index b676b7bde..6788937c6 100644
--- a/src/dns.cpp
+++ b/src/dns.cpp
@@ -724,6 +724,7 @@ std::string DNS::GetResult()
 {
         result = dns_getresult(this->fd);
         if (result) {
+		dns_close(this->fd);
 		return result;
         } else {
 		return "";
diff --git a/src/inspircd.cpp b/src/inspircd.cpp
index 2c67995e3..af4e741ac 100644
--- a/src/inspircd.cpp
+++ b/src/inspircd.cpp
@@ -98,7 +98,6 @@ bool AllowFounder = true;
 extern std::vector<Module*> modules;
 std::vector<std::string> module_names;
 extern std::vector<ircd_module*> factory;
-std::vector<int> fd_reap;
 
 extern int MODCOUNT;
 int openSockfd[MAXSOCKS];
@@ -2049,11 +2048,13 @@ void kill_link(userrec *user,const char* r)
 		NetSendToAll(buffer);
 	}
 
-	/* push the socket on a stack of sockets due to be closed at the next opportunity
-	 * 'Client exited' is an exception to this as it means the client side has already
-	 * closed the socket, we don't need to do it.
-	 */
-	fd_reap.push_back(user->fd);
+	FOREACH_MOD OnUserDisconnect(user);
+
+	if (user->fd > -1)
+	{
+		shutdown(user->fd,2);
+		close(user->fd);
+	}
 	
 	bool do_purge = false;
 	
@@ -2105,11 +2106,13 @@ void kill_link_silent(userrec *user,const char* r)
 		NetSendToAll(buffer);
 	}
 
-	/* push the socket on a stack of sockets due to be closed at the next opportunity
-	 * 'Client exited' is an exception to this as it means the client side has already
-	 * closed the socket, we don't need to do it.
-	 */
-	fd_reap.push_back(user->fd);
+	FOREACH_MOD OnUserDisconnect(user);
+
+        if (user->fd > -1)
+        {
+                shutdown(user->fd,2);
+                close(user->fd);
+        }
 	
 	bool do_purge = false;
 	
@@ -3448,7 +3451,6 @@ void RemoveServer(const char* name)
 }
 
 
-int reap_counter = 0;
 char MODERR[MAXBUF];
 
 char* ModuleError()
@@ -3822,23 +3824,6 @@ int InspIRCd(void)
 		}
 		if ((TIME % 5) == 1)
 			expire_run = false;
-		if (reap_counter>300)
-  		{
-			if (fd_reap.size() > 0)
-   			{
-				for( int n = 0; n < fd_reap.size(); n++)
-				{
-					if ((fd_reap[n] > -1))
-					{
-						close(fd_reap[n]);
-						shutdown (fd_reap[n],2);
-					}
-				}
-			}
-			fd_reap.clear();
-			reap_counter=0;
-		}
-		reap_counter++;
 		
 		// fix by brain - this must be below any manipulation of the hashmap by modules
 		user_hash::iterator count2 = clientlist.begin();
diff --git a/src/modules.cpp b/src/modules.cpp
index 990e23d0b..ca167bef1 100644
--- a/src/modules.cpp
+++ b/src/modules.cpp
@@ -309,6 +309,7 @@ Module::Module() { }
 Module::~Module() { }
 void Module::OnUserConnect(userrec* user) { }
 void Module::OnUserQuit(userrec* user) { }
+void Module::OnUserDisconnect(userrec* user) { }
 void Module::OnUserJoin(userrec* user, chanrec* channel) { }
 void Module::OnUserPart(userrec* user, chanrec* channel) { }
 void Module::OnPacketTransmit(std::string &data, std::string serv) { }
diff --git a/src/modules/m_ident.cpp b/src/modules/m_ident.cpp
index 90a6f448a..bf4e66a6c 100644
--- a/src/modules/m_ident.cpp
+++ b/src/modules/m_ident.cpp
@@ -69,6 +69,16 @@ class RFC1413
 	bool timeout;		// true if we've timed out and should bail
  public:
 
+	~RFC1413()
+	{
+		if (this->fd != -1)
+		{
+			shutdown(this->fd,2);
+			close(this->fd);
+			this->fd = -1;
+		}
+	}
+
 	// establish an ident connection, maxtime is the time to spend trying
 	// returns true if successful, false if something was catastrophically wrong.
 	// note that failed connects are not reported here but detected in RFC1413::Poll()
@@ -100,8 +110,9 @@ class RFC1413
 			{
 				// ... so that error isnt fatal, like the rest.
 				Srv->Log(DEBUG,"Ident: connect failed for: "+std::string(user->ip));
+				shutdown(this->fd,2);
                                 close(this->fd);
-                                shutdown(this->fd,2);
+				this->fd = -1;
 	                        return false;
 			}
                 }
@@ -120,8 +131,9 @@ class RFC1413
 		{
 			timeout = true;
 			Srv->SendServ(u->fd,"NOTICE "+std::string(u->nick)+" :*** Could not find your ident, using "+std::string(u->ident)+" instead.");
-			close(this->fd);
 			shutdown(this->fd,2);
+			close(this->fd);
+			this->fd = -1;
 			return false;
 		}
 		pollfd polls;
@@ -151,8 +163,9 @@ class RFC1413
 					if ((getsockname(this->u->fd,(sockaddr*)&sock_us,&uslen) || getpeername(this->u->fd, (sockaddr*)&sock_them, &themlen)))
 					{
 						Srv->Log(DEBUG,"Ident: failed to get socket names, bailing to state 3");
+						shutdown(this->fd,2);
                                                 close(this->fd);
-                                                shutdown(this->fd,2);
+						this->fd = -1;
 						state = IDENT_STATE_DONE;
 					}
 					else
@@ -173,8 +186,9 @@ class RFC1413
 						// 6195, 23 : ERROR : NO-USER
 						ibuf[nrecv] = '\0';
 						Srv->Log(DEBUG,"Received ident response: "+std::string(ibuf));
-						close(this->fd);
 						shutdown(this->fd,2);
+						close(this->fd);
+						this->fd = -1;
 						char* savept;
 						char* section = strtok_r(ibuf,":",&savept);
 						while (section)
@@ -209,6 +223,9 @@ class RFC1413
 					}
 				break;
 				case IDENT_STATE_DONE:
+					shutdown(this->fd,2);
+					close(this->fd);
+					this->fd = -1;
 					Srv->Log(DEBUG,"Ident lookup is complete!");
 				break;
 				default:
@@ -223,7 +240,7 @@ class RFC1413
 
 	bool Done()
 	{
-		return ((state == 3) || (timeout == true));
+		return ((state == IDENT_STATE_DONE) || (timeout == true));
 	}
 };
 
@@ -297,6 +314,18 @@ class ModuleIdent : public Module
 		}
 		return true;
 	}
+
+        virtual void OnUserDisconnect(userrec* user)
+        {
+                // when the user quits tidy up any ident lookup they have pending to keep things tidy
+                // and to prevent a memory and FD leaks
+		RFC1413* ident = (RFC1413*)user->GetExt("ident_data");
+                if (ident)
+                {
+                        delete ident;
+                        user->Shrink("ident_data");
+                }
+        }
 	
 	virtual ~ModuleIdent()
 	{