[cgutman] 48624: [OSKITTCP] - Prevent multiple wakeups for the same event which caused nasty problems for the SEL_FIN event because we dereferenced our connection context 3 times which not only cau...

Author: cgutman
Date: Thu Aug 26 02:29:38 2010
New Revision: 48624

URL: http://svn.reactos.org/svn/reactos?rev=48624&view=rev
Log:
[OSKITTCP]
- Prevent multiple wakeups for the same event which caused nasty problems for the SEL_FIN event because we dereferenced our connection context 3 times which not only caused the connection endpoint to be freed while holding its spin lock but made the reference count negative
[TCPIP]
- Disassociate the address file from the connection endpoint before dereferencing/closing it to avoid a double dereference of the address file (not as harmful in this case as in the connection endpoint case)
[IP]
- Dereference the connection endpoint again if it was associated with an address file as the connection endpoint to fix a reference leak

Modified:
    trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c
    trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c
    trunk/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c

Modified: trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip...
==============================================================================
--- trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] Thu Aug 26 02:29:38 2010
@@ -379,9 +379,15 @@
   LockObject(AddrFile, &OldIrql);
   /* We have to close this connection because we started it */
   if( AddrFile->Listener )
+  {
+      AddrFile->Listener->AddressFile = NULL;
       TCPClose( AddrFile->Listener );
+  }
   if( AddrFile->Connection )
+  {
+      AddrFile->Connection->AddressFile = NULL;
       DereferenceObject( AddrFile->Connection );
+  }
   UnlockObject(AddrFile, OldIrql);

DereferenceObject(AddrFile);

Modified: trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/transport/tc...
==============================================================================
--- trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] Thu Aug 26 02:29:38 2010
@@ -234,7 +234,10 @@

/* If the socket is dead, remove the reference we added for oskit */
        if (Connection->SignalState & SEL_FIN)
+       {
+           Connection->SocketContext = NULL;
            DereferenceObject(Connection);
+       }
 }

VOID ConnectionFree(PVOID Object) {
@@ -663,17 +666,15 @@
     KIRQL OldIrql;
     NTSTATUS Status;
     PVOID Socket;
-
-    /* We don't rely on SocketContext == NULL for socket
-     * closure anymore but we still need it to determine
-     * if we caused the closure
-     */
+    PADDRESS_FILE AddressFile = NULL;
+    PCONNECTION_ENDPOINT AddressConnection = NULL;
+
     LockObject(Connection, &OldIrql);
     Socket = Connection->SocketContext;
     Connection->SocketContext = NULL;

/* Don't try to close again if the other side closed us already */
-    if (!(Connection->SignalState & SEL_FIN))
+    if (Socket)
     {
        /* We need to close here otherwise oskit will never indicate
         * SEL_FIN and we will never fully close the connection */
@@ -693,11 +694,26 @@
     }

if (Connection->AddressFile)
-        DereferenceObject(Connection->AddressFile);
+    {
+        LockObjectAtDpcLevel(Connection->AddressFile);
+        if (Connection->AddressFile->Connection == Connection)
+        {
+            AddressConnection = Connection->AddressFile->Connection;
+            Connection->AddressFile->Connection = NULL;
+        }
+        UnlockObjectFromDpcLevel(Connection->AddressFile);
+
+        AddressFile = Connection->AddressFile;
+        Connection->AddressFile = NULL;
+    }

UnlockObject(Connection, OldIrql);

DereferenceObject(Connection);
+    if (AddressConnection)
+        DereferenceObject(AddressConnection);
+    if (AddressFile)
+        DereferenceObject(AddressFile);

return Status;
 }

Modified: trunk/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/oskittcp/oskitt...
==============================================================================
--- trunk/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c [iso-8859-1] Thu Aug 26 02:29:38 2010
@@ -118,8 +118,10 @@
    	wakeup(so, (caddr_t)&head->so_timeo);
    } else {
    	wakeup(so, (caddr_t)&so->so_timeo);
+#ifndef __REACTOS__
    	sorwakeup(so);
    	sowwakeup(so);
+#endif
    }
 }

@@ -131,8 +133,10 @@
     so->so_state &= ~SS_ISCONNECTING;
     so->so_state |= (SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE);
     wakeup(so, (caddr_t)&so->so_timeo);
+#ifndef __REACTOS__
     sowwakeup(so);
     sorwakeup(so);
+#endif
 }

void
@@ -144,8 +148,10 @@
     so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
     so->so_state |= (SS_CANTRCVMORE|SS_CANTSENDMORE);
     wakeup(so, (caddr_t)&so->so_timeo);
+#ifndef __REACTOS__
     sowwakeup(so);
     sorwakeup(so);
+#endif
 }

/*
@@ -192,7 +198,9 @@
    	return ((struct socket *)0);
    }
    if (connstatus) {
+#ifndef __REACTOS__
    	sorwakeup(head);
+#endif
    	wakeup(head, (caddr_t)&head->so_timeo);
    	so->so_state |= connstatus;
    }