[ros-diffs] [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...

[cgutman at svn.reactos.org]

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/fileobjs.c?rev=48624&r1=48623&r2=48624&view=diff
==============================================================================
--- 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/tcp/tcp.c?rev=48624&r1=48623&r2=48624&view=diff
==============================================================================
--- 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/oskittcp/uipc_socket2.c?rev=48624&r1=48623&r2=48624&view=diff
==============================================================================
--- 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;
 	}