Cleanup and optimization in realmd netcode.

- Get rid of some potential memory leaks and lingering connections issues.
- Code style cleanup where applicable.

Please give proper feedback and let me know how she fares.

--HG--
branch : trunk

4 files changed, 113 insertions, 97 deletions

diff --git a/src/trinityrealm/Main.cpp b/src/trinityrealm/Main.cpp
index 92cfc4edbe6..9d7669864d9 100644
--- a/src/trinityrealm/Main.cpp
+++ b/src/trinityrealm/Main.cpp
@@ -202,13 +202,13 @@ extern int main(int argc, char **argv)
 
     ///- Launch the listening network socket
     RealmAcceptor acceptor;
-
+    
     uint16 rmport = sConfig.GetIntDefault("RealmServerPort", DEFAULT_REALMSERVER_PORT);
     std::string bind_ip = sConfig.GetStringDefault("BindIP", "0.0.0.0");
 
     ACE_INET_Addr bind_addr(rmport, bind_ip.c_str());
 
-    if(acceptor.open(bind_addr, ACE_Reactor::instance(), ACE_NONBLOCK) == -1)
+    if (acceptor.open(bind_addr, ACE_Reactor::instance(), ACE_NONBLOCK) == -1)
     {
         sLog.outError("Trinity realm can not bind to %s:%d", bind_ip.c_str(), rmport);
         return 1;
diff --git a/src/trinityrealm/RealmAcceptor.h b/src/trinityrealm/RealmAcceptor.h
index 14a52134a28..5e243ea915b 100644
--- a/src/trinityrealm/RealmAcceptor.h
+++ b/src/trinityrealm/RealmAcceptor.h
@@ -43,7 +43,7 @@ class RealmAcceptor : public ACE_Acceptor<RealmSocket, ACE_SOCK_Acceptor>
             if (sh == 0)
                 ACE_NEW_RETURN(sh, RealmSocket, -1);
 
-            sh->reactor(this->reactor());
+            sh->reactor(reactor());
             sh->set_session(new AuthSocket(*sh));
             return 0;
         }
diff --git a/src/trinityrealm/RealmSocket.cpp b/src/trinityrealm/RealmSocket.cpp
index dbd957ec12e..db9845087a8 100644
--- a/src/trinityrealm/RealmSocket.cpp
+++ b/src/trinityrealm/RealmSocket.cpp
@@ -24,6 +24,8 @@
 
 #include "RealmSocket.h"
 
+#include "Log.h"
+
 #include <ace/OS_NS_string.h>
 #include <ace/INET_Addr.h>
 #include <ace/SString.h>
@@ -41,111 +43,128 @@ RealmSocket::Session::~Session(void)
 }
 
 RealmSocket::RealmSocket(void):
-    input_buffer_(4096),
     session_(NULL),
+    input_buffer_(4096),
     remote_address_()
 {
-    this->reference_counting_policy().value(
+    reference_counting_policy().value(
             ACE_Event_Handler::Reference_Counting_Policy::ENABLED);
 
-    this->msg_queue()->high_water_mark(8*1024*1024);
-    this->msg_queue()->low_water_mark(8*1024*1024);
+    msg_queue()->high_water_mark(8*1024*1024);
+    msg_queue()->low_water_mark(8*1024*1024);
+    
 }
 
-/*virtual*/ RealmSocket::~RealmSocket(void)
+RealmSocket::~RealmSocket(void)
 {
-    if(this->session_)
-        delete this->session_;
-
-    if(this->peer().get_handle() != ACE_INVALID_HANDLE)
-        this->peer().close();
+    if (msg_queue())
+        msg_queue()->close();
+
+    if (input_buffer_.length() != 0)
+        input_buffer_.release();
+        
+    // delete RealmSocketObject must never be called from our code.
+    closing_ = true;
+
+    if (session_);
+        delete esession_;
+    
+    peer().close();
 }
 
-/*virtual*/ int RealmSocket::open(void * arg)
+int RealmSocket::open(void * arg)
 {
-    if(Base::open(arg) == -1)
-        return -1;
-
     ACE_INET_Addr addr;
 
-    if(peer().get_remote_addr(addr) == -1)
+    if (peer ().get_remote_addr (addr) == -1)
+    {
+        sLog.outError ("RealmSocket::open: peer ().get_remote_addr errno = %s", ACE_OS::strerror (errno));
         return -1;
+    }
 
-    char address[1024];
+    remote_address_ = addr.get_host_addr();
+    
+    // Register with ACE Reactor
+    if (Base::open(arg) == -1)
+        return -1;
 
-    addr.get_host_addr(address, 1024);
+    if (session_ != NULL)
+    {
+        session_->OnAccept();
+    }
 
-    this->remote_address_ = address;
+    // reactor takes care of the socket from now on
+    remove_reference();
 
-    if(this->session_ != NULL)
-    {
-        // Prepare for upcall
-        this->add_reference();
-        ACE_Event_Handler_var guard(this);
+    return 0;
+}
 
-        this->session_->OnAccept();
-    }
+int RealmSocket::close(int)
+{
+    shutdown();
+    
+    closing_ = true;
 
-    this->remove_reference();
+    remove_reference();
 
     return 0;
 }
 
 const ACE_CString& RealmSocket::get_remote_address(void) const
 {
-    return this->remote_address_;
+    return remote_address_;
 }
 
 size_t RealmSocket::recv_len(void) const
 {
-    return this->input_buffer_.length();
+    return input_buffer_.length();
 }
 
 bool RealmSocket::recv_soft(char *buf, size_t len)
 {
-    if(this->input_buffer_.length() < len)
+    if (input_buffer_.length() < len)
         return false;
 
-    ACE_OS::memcpy(buf, this->input_buffer_.rd_ptr(), len);
+    ACE_OS::memcpy(buf, input_buffer_.rd_ptr(), len);
 
     return true;
 }
 
 bool RealmSocket::recv(char *buf, size_t len)
 {
-    bool ret = this->recv_soft(buf, len);
+    bool ret = recv_soft(buf, len);
 
-    if(ret)
-        this->recv_skip(len);
+    if (ret)
+        recv_skip(len);
 
     return ret;
 }
 
 void RealmSocket::recv_skip(size_t len)
 {
-    this->input_buffer_.rd_ptr(len);
+    input_buffer_.rd_ptr(len);
 }
 
 ssize_t RealmSocket::noblk_send(ACE_Message_Block &message_block)
 {
     const size_t len = message_block.length();
 
-    if(len == 0)
+    if (len == 0)
         return -1;
 
     // Try to send the message directly.
-    ssize_t n = this->peer().send(message_block.rd_ptr(), len, MSG_NOSIGNAL);
+    ssize_t n = peer().send(message_block.rd_ptr(), len, MSG_NOSIGNAL);
 
-    if(n < 0)
+    if (n < 0)
     {
-        if(errno == EWOULDBLOCK)
+        if (errno == EWOULDBLOCK)
             // Blocking signal
             return 0;
         else
             // Error happened
             return -1;
     }
-    else if(n == 0)
+    else if (n == 0)
     {
         // Can this happen ?
         return -1;
@@ -157,7 +176,7 @@ ssize_t RealmSocket::noblk_send(ACE_Message_Block &message_block)
 
 bool RealmSocket::send(const char *buf, size_t len)
 {
-    if(buf == NULL || len == 0)
+    if (buf == NULL || len == 0)
         return true;
 
     ACE_Data_Block db(
@@ -176,14 +195,14 @@ bool RealmSocket::send(const char *buf, size_t len)
 
     message_block.wr_ptr(len);
 
-    if(this->msg_queue()->is_empty())
+    if (msg_queue()->is_empty())
     {
         // Try to send it directly.
-        ssize_t n = this->noblk_send(message_block);
+        ssize_t n = noblk_send(message_block);
 
-        if(n < 0)
+        if (n < 0)
             return false;
-        else if(n == len)
+        else if (n == len)
             return true;
 
         // fall down
@@ -192,39 +211,42 @@ bool RealmSocket::send(const char *buf, size_t len)
 
     ACE_Message_Block *mb = message_block.clone();
 
-    if(this->msg_queue()->enqueue_tail(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
+    if (msg_queue()->enqueue_tail(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
     {
         mb->release();
         return false;
     }
 
-    if(this->reactor()->schedule_wakeup(this, ACE_Event_Handler::WRITE_MASK) == -1)
+    if (reactor()->schedule_wakeup(this, ACE_Event_Handler::WRITE_MASK) == -1)
         return false;
 
     return true;
 }
 
-/*virtual*/ int RealmSocket::handle_output(ACE_HANDLE /*= ACE_INVALID_HANDLE*/)
+int RealmSocket::handle_output(ACE_HANDLE /*= ACE_INVALID_HANDLE*/)
 {
+    if (closing_)
+        return -1;
+
     ACE_Message_Block *mb = 0;
 
-    if(this->msg_queue()->is_empty())
+    if (msg_queue()->is_empty())
     {
-        this->reactor()->cancel_wakeup(this, ACE_Event_Handler::WRITE_MASK);
+        reactor()->cancel_wakeup(this, ACE_Event_Handler::WRITE_MASK);
         return 0;
     }
 
-    if(this->msg_queue()->dequeue_head(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
+    if (msg_queue()->dequeue_head(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
         return -1;
 
-    ssize_t n = this->noblk_send(*mb);
+    ssize_t n = noblk_send(*mb);
 
-    if(n < 0)
+    if (n < 0)
     {
         mb->release();
         return -1;
     }
-    else if(n == mb->length())
+    else if (n == mb->length())
     {
         mb->release();
         return 1;
@@ -233,7 +255,7 @@ bool RealmSocket::send(const char *buf, size_t len)
     {
         mb->rd_ptr(n);
 
-        if(this->msg_queue()->enqueue_head(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
+        if (msg_queue()->enqueue_head(mb, (ACE_Time_Value *) &ACE_Time_Value::zero) == -1)
         {
             mb->release();
             return -1;
@@ -245,68 +267,61 @@ bool RealmSocket::send(const char *buf, size_t len)
     ACE_NOTREACHED(return -1);
 }
 
-/*virtual*/ int RealmSocket::handle_input(ACE_HANDLE /*= ACE_INVALID_HANDLE*/)
+int RealmSocket::handle_close(ACE_HANDLE h, ACE_Reactor_Mask m)
 {
-    const ssize_t space = this->input_buffer_.space();
+    // As opposed to WorldSocket::handle_close, we don't need locks here.
 
-    ssize_t n = this->peer().recv(this->input_buffer_.wr_ptr(), space);
+    closing_ = true;
 
-    if(n < 0)
+    if (h == ACE_INVALID_HANDLE)
+        peer ().close_writer ();
+            
+    if (session_ != NULL)
+    {
+        session_->OnClose();
+    }
+
+    return 0;
+}
+
+int RealmSocket::handle_input(ACE_HANDLE /*= ACE_INVALID_HANDLE*/)
+{
+    if (closing_)
+        return -1;
+
+    const ssize_t space = input_buffer_.space();
+
+    ssize_t n = peer().recv(input_buffer_.wr_ptr(), space);
+
+    if (n < 0)
     {
         return errno == EWOULDBLOCK ? 0 : -1;
     }
-    else if(n == 0)
+    else if (n == 0)
     {
         // EOF
         return -1;
     }
 
-    this->input_buffer_.wr_ptr((size_t)n);
+    input_buffer_.wr_ptr((size_t)n);
 
-    if(this->session_ != NULL)
+    if (session_ != NULL)
     {
-        // Prepare for upcall
-        this->add_reference();
-        ACE_Event_Handler_var guard(this);
-
-        this->session_->OnRead();
+        session_->OnRead();
 
-        this->input_buffer_.crunch();
+        input_buffer_.crunch();
     }
 
     // return 1 in case there is more data to read from OS
     return n == space ? 1 : 0;
 }
 
-/*virtual*/ int RealmSocket::handle_close(ACE_HANDLE h, ACE_Reactor_Mask m)
-{
-    if(this->session_ != NULL)
-    {
-        // Prepare for upcall
-        this->add_reference();
-        ACE_Event_Handler_var guard(this);
-
-        this->session_->OnClose();
-    }
-
-    this->shutdown();
-
-    return 0;
-}
 
 void RealmSocket::set_session(Session* session)
 {
-    if(this->session_ != NULL)
-        delete this->session_;
+    if (session_ != NULL)
+        delete session_;
 
-    this->session_ = session;
+    session_ = session;
 }
 
-int RealmSocket::close(int)
-{
-    this->shutdown();
-
-    this->remove_reference();
-
-    return 0;
-}
diff --git a/src/trinityrealm/RealmSocket.h b/src/trinityrealm/RealmSocket.h
index 2ac021d5cc3..13be8327533 100644
--- a/src/trinityrealm/RealmSocket.h
+++ b/src/trinityrealm/RealmSocket.h
@@ -63,6 +63,8 @@ class RealmSocket : public ACE_Svc_Handler<ACE_SOCK_STREAM, ACE_NULL_SYNCH>
 
         virtual int open(void *);
 
+        virtual int close(int);
+
         virtual int handle_input(ACE_HANDLE = ACE_INVALID_HANDLE);
         virtual int handle_output(ACE_HANDLE = ACE_INVALID_HANDLE);
 
@@ -71,7 +73,6 @@ class RealmSocket : public ACE_Svc_Handler<ACE_SOCK_STREAM, ACE_NULL_SYNCH>
 
         void set_session(Session* session);
 
-        int close(int);
     private:
         ssize_t noblk_send(ACE_Message_Block &message_block);