Make error reporting work properly, it seemed to loose errors.

First change: Add a culling list of inspsockets waiting to be closed, so the close() is less likely to be called before the buffer is entirely empty. This seems to work well on my LAN.
Second change: Add a SendError() method, rather than WriteLine("ERROR : ..."). This can be used to effect by also echoing out "Sent ERROR to %s: ..." onto the +l snomask meaning at least one end
will always see the error even if the ERROR command is lost due to latency or design of the transport (e.g. ssl which may be writing during a read event etc)

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

diff --git a/include/inspircd.h b/include/inspircd.h
index bfd2ea4212161cab60bb6f808c049610491fc598..d9918aa737087fd0b466d47e99936c59bcc6f544 100644 (file)
--- a/include/inspircd.h
+++ b/include/inspircd.h
@@ -432,6 +432,8 @@ class InspIRCd : public classbase
 
  public:
 
+       std::map<InspSocket*,InspSocket*> SocketCull;
+
        /** Build the ISUPPORT string by triggering all modules On005Numeric events
         */
        void BuildISupport();

diff --git a/src/inspircd.cpp b/src/inspircd.cpp
index bacd92961bded86a501414155796e807f68ebe85..670b244415a2d00f747a274598848e4310a56acb 100644 (file)
--- a/src/inspircd.cpp
+++ b/src/inspircd.cpp
@@ -911,6 +911,14 @@ void InspIRCd::DoOneIteration(bool process_module_sockets)
        /* if any users was quit, take them out */
        GlobalCulls.Apply();
 
+       /* If any inspsockets closed, remove them */
+       for (std::map<InspSocket*,InspSocket*>::iterator x = SocketCull.begin(); x != SocketCull.end(); ++x)
+       {
+               SE->DelFd(x->second);
+               x->second->Close();
+               delete x->second;
+       }
+       SocketCull.clear();
 }
 
 bool InspIRCd::IsIdent(const char* n)
@@ -933,7 +941,6 @@ bool InspIRCd::IsIdent(const char* n)
        return true;
 }
 
-
 int InspIRCd::Run()
 {
        while (true)

diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp
index 5641ba6ef37b1a4c422b61b5cf13b2625f5bf48a..3c30fd6e80387991162ec5a4cacf4e427aeacf50 100644 (file)
--- a/src/inspsocket.cpp
+++ b/src/inspsocket.cpp
@@ -573,14 +573,16 @@ void SocketTimeout::Tick(time_t now)
                this->sock->OnTimeout();
                this->sock->OnError(I_ERR_TIMEOUT);
                this->sock->timeout = true;
-               ServerInstance->SE->DelFd(this->sock);
+
                /* NOTE: We must set this AFTER DelFd, as we added
                 * this socket whilst writeable. This means that we
                 * must DELETE the socket whilst writeable too!
                 */
                this->sock->state = I_ERROR;
-               this->sock->Close();
-               delete this->sock;
+
+               if (ServerInstance->SocketCull.find(this->sock) == ServerInstance->SocketCull.end())
+                       ServerInstance->SocketCull[this->sock] = this->sock;
+
                return;
        }
        else
@@ -717,17 +719,15 @@ void InspSocket::HandleEvent(EventType et, int errornum)
        switch (et)
        {
                case EVENT_ERROR:
-                       this->Instance->SE->DelFd(this);
-                       this->Close();
-                       delete this;
+                       if (this->Instance->SocketCull.find(this) == this->Instance->SocketCull.end())
+                               this->Instance->SocketCull[this] = this;
                        return;
                break;
                case EVENT_READ:
                        if (!this->Poll())
                        {
-                               this->Instance->SE->DelFd(this);
-                               this->Close();
-                               delete this;
+                               if (this->Instance->SocketCull.find(this) == this->Instance->SocketCull.end())
+                                       this->Instance->SocketCull[this] = this;
                                return;
                        }
                break;
@@ -737,9 +737,8 @@ void InspSocket::HandleEvent(EventType et, int errornum)
                                this->WaitingForWriteEvent = false;
                                if (!this->OnWriteReady())
                                {
-                                       this->Instance->SE->DelFd(this);
-                                       this->Close();
-                                       delete this;
+                                       if (this->Instance->SocketCull.find(this) == this->Instance->SocketCull.end())
+                                               this->Instance->SocketCull[this] = this;
                                        return;
                                }
                        }
@@ -758,9 +757,8 @@ void InspSocket::HandleEvent(EventType et, int errornum)
                        {
                                if (this->FlushWriteBuffer())
                                {
-                                       this->Instance->SE->DelFd(this);
-                                       this->Close();
-                                       delete this;
+                                       if (this->Instance->SocketCull.find(this) == this->Instance->SocketCull.end())
+                                               this->Instance->SocketCull[this] = this;
                                        return;
                                }
                        }

diff --git a/src/modules/m_spanningtree/treesocket.h b/src/modules/m_spanningtree/treesocket.h
index ff06f0926691a92e1f539ecb79128565d24a2885..15ca8f4cbabc8233c94bff4d332148f993f5e4d7 100644 (file)
--- a/src/modules/m_spanningtree/treesocket.h
+++ b/src/modules/m_spanningtree/treesocket.h
@@ -171,6 +171,11 @@ class TreeSocket : public InspSocket
         */
        virtual void OnError(InspSocketError e);
 
+       /** Sends an error to the remote server, and displays it locally to show
+        * that it was sent.
+        */
+       void SendError(const std::string &errormessage);
+
        /** Handle socket disconnect event
         */
        virtual int OnDisconnect();

diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp
index 21d6b912f388f815ea0fafa49d2e8b3dd5cfe3e3..a05ed6e4c2001fa4b76cb872859aeb031e8f53d3 100644 (file)
--- a/src/modules/m_spanningtree/treesocket1.cpp
+++ b/src/modules/m_spanningtree/treesocket1.cpp
@@ -373,11 +373,17 @@ std::string TreeSocket::ListDifference(const std::string &one, const std::string
        return result;
 }
 
+void TreeSocket::SendError(const std::string &errormessage)
+{
+       this->WriteLine("ERROR :"+errormessage);
+       this->Instance->SNO->WriteToSnoMask('l',"Sent \2ERROR\2 to "+this->InboundServerName+": "+errormessage);
+}
+
 bool TreeSocket::Capab(const std::deque<std::string> &params)
 {
        if (params.size() < 1)
        {
-               this->WriteLine("ERROR :Invalid number of parameters for CAPAB - Mismatched version");
+               this->SendError("Invalid number of parameters for CAPAB - Mismatched version");
                return false;
        }
        if (params[0] == "START")
@@ -467,7 +473,7 @@ bool TreeSocket::Capab(const std::deque<std::string> &params)
 
                if (reason.length())
                {
-                       this->WriteLine("ERROR :CAPAB negotiation failed: "+reason);
+                       this->SendError("CAPAB negotiation failed: "+reason);
                        return false;
                }
        }
@@ -843,7 +849,7 @@ bool TreeSocket::ForceJoin(const std::string &source, std::deque<std::string> &p
                                         * danger bill bobbertson! (that's will robinsons older brother ;-) ...)
                                         */
                                        this->Instance->WriteOpers("ERROR: We received a user with an unknown prefix '%c'. Closed connection to avoid a desync.",*permissions);
-                                       this->WriteLine(std::string("ERROR :Invalid prefix '")+(*permissions)+"' in FJOIN");
+                                       this->SendError(std::string("Invalid prefix '")+(*permissions)+"' in FJOIN");
                                        return false;
                                }
                                usr++;

diff --git a/src/modules/m_spanningtree/treesocket2.cpp b/src/modules/m_spanningtree/treesocket2.cpp
index 588b129613282ade0cbfc23bd1beb951ec76b784..397a7c0b0da58cdc833e2a0a8a1f0523f613b4dc 100644 (file)
--- a/src/modules/m_spanningtree/treesocket2.cpp
+++ b/src/modules/m_spanningtree/treesocket2.cpp
@@ -798,13 +798,13 @@ bool TreeSocket::RemoteServer(const std::string &prefix, std::deque<std::string>
        TreeServer* ParentOfThis = Utils->FindServer(prefix);
        if (!ParentOfThis)
        {
-               this->WriteLine("ERROR :Protocol error - Introduced remote server from unknown server "+prefix);
+               this->SendError("Protocol error - Introduced remote server from unknown server "+prefix);
                return false;
        }
        TreeServer* CheckDupe = Utils->FindServer(servername);
        if (CheckDupe)
        {
-               this->WriteLine("ERROR :Server "+servername+" already exists!");
+               this->SendError("Server "+servername+" already exists!");
                this->Instance->SNO->WriteToSnoMask('l',"Server \2"+servername+"\2 being introduced from \2" + prefix + "\2 denied, already exists. Closing link with " + prefix);
                return false;
        }
@@ -843,15 +843,19 @@ bool TreeSocket::Outbound_Reply_Server(std::deque<std::string> &params)
        irc::string servername = params[0].c_str();
        std::string sname = params[0];
        std::string password = params[1];
+       std::string description = params[3];
        int hops = atoi(params[2].c_str());
 
+       this->InboundServerName = sname;
+       this->InboundDescription = description;
+
        if (hops)
        {
-               this->WriteLine("ERROR :Server too far away for authentication");
+               this->SendError("Server too far away for authentication");
                this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, server is too far away for authentication");
                return false;
        }
-       std::string description = params[3];
+
        for (std::vector<Link>::iterator x = Utils->LinkBlocks.begin(); x < Utils->LinkBlocks.end(); x++)
        {
                if ((x->Name == servername) && ((ComparePass(this->MakePass(x->RecvPass,this->GetOurChallenge()),password)) || (x->RecvPass == password && (this->GetTheirChallenge().empty()))))
@@ -859,7 +863,7 @@ bool TreeSocket::Outbound_Reply_Server(std::deque<std::string> &params)
                        TreeServer* CheckDupe = Utils->FindServer(sname);
                        if (CheckDupe)
                        {
-                               this->WriteLine("ERROR :Server "+sname+" already exists on server "+CheckDupe->GetParent()->GetName()+"!");
+                               this->SendError("Server "+sname+" already exists on server "+CheckDupe->GetParent()->GetName()+"!");
                                this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, already exists on server "+CheckDupe->GetParent()->GetName());
                                return false;
                        }
@@ -880,7 +884,7 @@ bool TreeSocket::Outbound_Reply_Server(std::deque<std::string> &params)
                        return true;
                }
        }
-       this->WriteLine("ERROR :Invalid credentials");
+       this->SendError("Invalid credentials");
        this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, invalid link credentials");
        return false;
 }
@@ -892,15 +896,19 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
        irc::string servername = params[0].c_str();
        std::string sname = params[0];
        std::string password = params[1];
+       std::string description = params[3];
        int hops = atoi(params[2].c_str());
 
+       this->InboundServerName = sname;
+       this->InboundDescription = description;
+
        if (hops)
        {
-               this->WriteLine("ERROR :Server too far away for authentication");
+               this->SendError("Server too far away for authentication");
                this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, server is too far away for authentication");
                return false;
        }
-       std::string description = params[3];
+
        for (std::vector<Link>::iterator x = Utils->LinkBlocks.begin(); x < Utils->LinkBlocks.end(); x++)
        {
                if ((x->Name == servername) && ((ComparePass(this->MakePass(x->RecvPass,this->GetOurChallenge()),password) || x->RecvPass == password && (this->GetTheirChallenge().empty()))))
@@ -910,9 +918,9 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
                        if (CheckDupeSocket)
                        {
                                /* If we find one, we abort the link to prevent a race condition */
-                               this->WriteLine("ERROR :Negotiation collision");
+                               this->SendError("Negotiation collision");
                                this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, already exists in a negotiating state.");
-                               CheckDupeSocket->WriteLine("ERROR :Negotiation collision");
+                               CheckDupeSocket->SendError("Negotiation collision");
                                Instance->SE->DelFd(CheckDupeSocket);
                                CheckDupeSocket->Close();
                                delete CheckDupeSocket;
@@ -922,7 +930,7 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
                        TreeServer* CheckDupe = Utils->FindServer(sname);
                        if (CheckDupe)
                        {
-                               this->WriteLine("ERROR :Server "+sname+" already exists on server "+CheckDupe->GetParent()->GetName()+"!");
+                               this->SendError("Server "+sname+" already exists on server "+CheckDupe->GetParent()->GetName()+"!");
                                this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, already exists on server "+CheckDupe->GetParent()->GetName());
                                return false;
                        }
@@ -935,8 +943,6 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
 
                        Utils->AddBurstingServer(sname,this);
 
-                       this->InboundServerName = sname;
-                       this->InboundDescription = description;
                        // this is good. Send our details: Our server name and description and hopcount of 0,
                        // along with the sendpass from this block.
                        this->WriteLine(std::string("SERVER ")+this->Instance->Config->ServerName+" "+this->MakePass(x->SendPass, this->GetTheirChallenge())+" 0 :"+this->Instance->Config->ServerDesc);
@@ -945,7 +951,7 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
                        return true;
                }
        }
-       this->WriteLine("ERROR :Invalid credentials");
+       this->SendError("Invalid credentials");
        this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, invalid link credentials");
        return false;
 }
@@ -1006,7 +1012,7 @@ bool TreeSocket::ProcessLine(std::string &line)
                        }
                        else if (command == "USER")
                        {
-                               this->WriteLine("ERROR :Client connections to this port are prohibited.");
+                               this->SendError("Client connections to this port are prohibited.");
                                return false;
                        }
                        else if (command == "CAPAB")
@@ -1015,14 +1021,13 @@ bool TreeSocket::ProcessLine(std::string &line)
                        }
                        else if ((command == "U") || (command == "S"))
                        {
-                               this->WriteLine("ERROR :Cannot use the old-style mesh linking protocol with m_spanningtree.so!");
+                               this->SendError("Cannot use the old-style mesh linking protocol with m_spanningtree.so!");
                                return false;
                        }
                        else
                        {
-                               std::string error("ERROR :Invalid command in negotiation phase: ");
-                               error.append(command.c_str());
-                               this->WriteLine(error);
+                               irc::string error = "Invalid command in negotiation phase: " + command;
+                               this->SendError(assign(error));
                                return false;
                        }
                break;
@@ -1037,7 +1042,7 @@ bool TreeSocket::ProcessLine(std::string &line)
                        }
                        else if ((command == "U") || (command == "S"))
                        {
-                               this->WriteLine("ERROR :Cannot use the old-style mesh linking protocol with m_spanningtree.so!");
+                               this->SendError("Cannot use the old-style mesh linking protocol with m_spanningtree.so!");
                                return false;
                        }
                        else if (command == "BURST")
@@ -1050,7 +1055,7 @@ bool TreeSocket::ProcessLine(std::string &line)
                                        if ((delta < -300) || (delta > 300))
                                        {
                                                Instance->SNO->WriteToSnoMask('l',"\2ERROR\2: Your clocks are out by %d seconds (this is more than five minutes). Link aborted, \2PLEASE SYNC YOUR CLOCKS!\2",abs(delta));
-                                               WriteLine("ERROR :Your clocks are out by "+ConvToStr(abs(delta))+" seconds (this is more than ten minutes). Link aborted, PLEASE SYNC YOUR CLOCKS!");
+                                               SendError("Your clocks are out by "+ConvToStr(abs(delta))+" seconds (this is more than ten minutes). Link aborted, PLEASE SYNC YOUR CLOCKS!");
                                                return false;
                                        }
                                        else if ((delta < -30) || (delta > 30))
@@ -1090,7 +1095,7 @@ bool TreeSocket::ProcessLine(std::string &line)
 
                break;
                case LISTENER:
-                       this->WriteLine("ERROR :Internal error -- listening socket accepted its own descriptor!!!");
+                       this->SendError("Internal error -- listening socket accepted its own descriptor!!!");
                        return false;
                break;
                case CONNECTING:
@@ -1447,7 +1452,7 @@ bool TreeSocket::ProcessLine(std::string &line)
                                        switch (this->Instance->CallCommandHandler(command.c_str(), strparams, params.size(), who))
                                        {
                                                case CMD_INVALID:
-                                                       this->WriteLine("ERROR :Unrecognised command '"+std::string(command.c_str())+"' -- possibly loaded mismatched modules");
+                                                       this->SendError("Unrecognised command '"+std::string(command.c_str())+"' -- possibly loaded mismatched modules");
                                                        return false;
                                                break;
                                                case CMD_FAILURE: