[cgutman] 59705: [TCPIP] - Add logging of address files and connections (on temporarily for testing changes) [LWIP] - Drastically simplify the closing state machine - All connection objects (and as...
Author: cgutman Date: Mon Aug 12 03:09:28 2013 New Revision: 59705
URL: http://svn.reactos.org/svn/reactos?rev=59705&view=rev Log: [TCPIP] - Add logging of address files and connections (on temporarily for testing changes) [LWIP] - Drastically simplify the closing state machine - All connection objects (and as a result address files too) are now getting properly reaped now - Tested with Firefox and Abyss Web Server
Modified: trunk/reactos/drivers/network/tcpip/include/fileobjs.h trunk/reactos/drivers/network/tcpip/include/tcp.h trunk/reactos/drivers/network/tcpip/include/titypes.h trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c trunk/reactos/lib/drivers/ip/network/ip.c trunk/reactos/lib/drivers/lwip/src/rostcp.c
Modified: trunk/reactos/drivers/network/tcpip/include/fileobjs.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/inclu... ============================================================================== --- trunk/reactos/drivers/network/tcpip/include/fileobjs.h [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/include/fileobjs.h [iso-8859-1] Mon Aug 12 03:09:28 2013 @@ -35,4 +35,6 @@ NTSTATUS FileCloseControlChannel( PTDI_REQUEST Request);
+VOID LogActiveObjects(VOID); + /* EOF */
Modified: trunk/reactos/drivers/network/tcpip/include/tcp.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/inclu... ============================================================================== --- trunk/reactos/drivers/network/tcpip/include/tcp.h [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/include/tcp.h [iso-8859-1] Mon Aug 12 03:09:28 2013 @@ -211,3 +211,6 @@ FlushAllQueues(PCONNECTION_ENDPOINT Connection, NTSTATUS Status);
VOID CompleteBucket(PCONNECTION_ENDPOINT Connection, PTDI_BUCKET Bucket, const BOOLEAN Synchronous); + +void +LibTCPDumpPcb(PVOID SocketContext);
Modified: trunk/reactos/drivers/network/tcpip/include/titypes.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/inclu... ============================================================================== --- trunk/reactos/drivers/network/tcpip/include/titypes.h [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/include/titypes.h [iso-8859-1] Mon Aug 12 03:09:28 2013 @@ -281,6 +281,7 @@ BOOLEAN SendShutdown; BOOLEAN ReceiveShutdown; NTSTATUS ReceiveShutdownStatus; + BOOLEAN Closing;
struct _CONNECTION_ENDPOINT *Next; /* Next connection in address file list */ } CONNECTION_ENDPOINT, *PCONNECTION_ENDPOINT;
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] Mon Aug 12 03:09:28 2013 @@ -10,6 +10,8 @@
#include "precomp.h"
+/* Uncomment for logging of connections and address files every 10 seconds */ +#define LOG_OBJECTS
/* List of all address file objects managed by this driver */ LIST_ENTRY AddressFileListHead; @@ -97,6 +99,88 @@ }
return FALSE; +} + +VOID +LogActiveObjects(VOID) +{ +#ifdef LOG_OBJECTS + PLIST_ENTRY CurrentEntry; + KIRQL OldIrql; + PADDRESS_FILE AddrFile; + PCONNECTION_ENDPOINT Conn; + + DbgPrint("----------- TCP/IP Active Object Dump -------------\n"); + + TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql); + + CurrentEntry = AddressFileListHead.Flink; + while (CurrentEntry != &AddressFileListHead) + { + AddrFile = CONTAINING_RECORD(CurrentEntry, ADDRESS_FILE, ListEntry); + + DbgPrint("Address File (%s, %d, %d) @ 0x%p | Ref count: %d | Sharers: %d\n", + A2S(&AddrFile->Address), WN2H(AddrFile->Port), AddrFile->Protocol, + AddrFile, AddrFile->RefCount, AddrFile->Sharers); + DbgPrint("\tListener: "); + if (AddrFile->Listener == NULL) + DbgPrint("<None>\n"); + else + DbgPrint("0x%p\n", AddrFile->Listener); + DbgPrint("\tAssociated endpoints: "); + if (AddrFile->Connection == NULL) + DbgPrint("<None>\n"); + else + { + Conn = AddrFile->Connection; + while (Conn) + { + DbgPrint("0x%p ", Conn); + Conn = Conn->Next; + } + DbgPrint("\n"); + } + + CurrentEntry = CurrentEntry->Flink; + } + + TcpipReleaseSpinLock(&AddressFileListLock, OldIrql); + + TcpipAcquireSpinLock(&ConnectionEndpointListLock, &OldIrql); + + CurrentEntry = ConnectionEndpointListHead.Flink; + while (CurrentEntry != &ConnectionEndpointListHead) + { + Conn = CONTAINING_RECORD(CurrentEntry, CONNECTION_ENDPOINT, ListEntry); + + DbgPrint("Connection @ 0x%p | Ref count: %d\n", Conn, Conn->RefCount); + DbgPrint("\tPCB: "); + if (Conn->SocketContext == NULL) + DbgPrint("<None>\n"); + else + { + DbgPrint("0x%p\n", Conn->SocketContext); + LibTCPDumpPcb(Conn->SocketContext); + } + DbgPrint("\tPacket queue status: %s\n", IsListEmpty(&Conn->PacketQueue) ? "Empty" : "Not Empty"); + DbgPrint("\tRequest lists: Connect: %s | Recv: %s | Send: %s | Shutdown: %s | Listen: %s\n", + IsListEmpty(&Conn->ConnectRequest) ? "Empty" : "Not Empty", + IsListEmpty(&Conn->ReceiveRequest) ? "Empty" : "Not Empty", + IsListEmpty(&Conn->SendRequest) ? "Empty" : "Not Empty", + IsListEmpty(&Conn->ShutdownRequest) ? "Empty" : "Not Empty", + IsListEmpty(&Conn->ListenRequest) ? "Empty" : "Not Empty"); + DbgPrint("\tSend shutdown: %s\n", Conn->SendShutdown ? "Yes" : "No"); + DbgPrint("\tReceive shutdown: %s\n", Conn->ReceiveShutdown ? "Yes" : "No"); + if (Conn->ReceiveShutdown) DbgPrint("\tReceive shutdown status: 0x%x\n", Conn->ReceiveShutdownStatus); + DbgPrint("\tClosing: %s\n", Conn->Closing ? "Yes" : "No"); + + CurrentEntry = CurrentEntry->Flink; + } + + TcpipReleaseSpinLock(&ConnectionEndpointListLock, OldIrql); + + DbgPrint("---------------------------------------------------\n"); +#endif }
PADDRESS_FILE AddrFindShared(
Modified: trunk/reactos/lib/drivers/ip/network/ip.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/ip.c... ============================================================================== --- trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] (original) +++ trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] Mon Aug 12 03:09:28 2013 @@ -23,6 +23,8 @@ /* Work around calling timer at Dpc level */
IP_PROTOCOL_HANDLER ProtocolTable[IP_PROTOCOL_TABLE_SIZE]; + +ULONG IpTimerExpirations;
VOID TCPRegisterInterface(PIP_INTERFACE IF); @@ -119,9 +121,16 @@ * SystemArgument1 = Unused * SystemArgument2 = Unused * NOTES: - * This routine is dispatched once in a while to do maintainance jobs - */ -{ + * This routine is dispatched once in a while to do maintenance jobs + */ +{ + IpTimerExpirations++; + + if ((IpTimerExpirations % 10) == 0) + { + LogActiveObjects(); + } + /* Check if datagram fragments have taken too long to assemble */ IPDatagramReassemblyTimeout();
Modified: trunk/reactos/lib/drivers/lwip/src/rostcp.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/lwip/src/rostcp... ============================================================================== --- trunk/reactos/lib/drivers/lwip/src/rostcp.c [iso-8859-1] (original) +++ trunk/reactos/lib/drivers/lwip/src/rostcp.c [iso-8859-1] Mon Aug 12 03:09:28 2013 @@ -35,6 +35,21 @@ /* Required for ERR_T to NTSTATUS translation in receive error handling */ NTSTATUS TCPTranslateError(const err_t err);
+void +LibTCPDumpPcb(PVOID SocketContext) +{ + struct tcp_pcb *pcb = (struct tcp_pcb*)SocketContext; + unsigned int addr = ntohl(pcb->remote_ip.addr); + + DbgPrint("\tState: %s\n", tcp_state_str[pcb->state]); + DbgPrint("\tRemote: (%d.%d.%d.%d, %d)\n", + (addr >> 24) & 0xFF, + (addr >> 16) & 0xFF, + (addr >> 8) & 0xFF, + addr & 0xFF, + pcb->remote_port); +} + static void LibTCPEmptyQueue(PCONNECTION_ENDPOINT Connection) @@ -231,18 +246,15 @@ Connection->ReceiveShutdown = TRUE; Connection->ReceiveShutdownStatus = STATUS_SUCCESS;
- /* This code path executes for both remotely and locally initiated closures, - * and we need to distinguish between them */ - if (Connection->SocketContext) + /* If we already did a send shutdown, we're in TIME_WAIT so we can't use this PCB anymore */ + if (Connection->SendShutdown) { - /* Remotely initiated close */ - TCPRecvEventHandler(arg); + Connection->SocketContext = NULL; + tcp_arg(pcb, NULL); } - else - { - /* Locally initated close */ - TCPFinEventHandler(arg, ERR_CLSD); - } + + /* Remotely initiated close */ + TCPRecvEventHandler(arg); }
return ERR_OK; @@ -285,30 +297,18 @@ InternalErrorEventHandler(void *arg, const err_t err) { PCONNECTION_ENDPOINT Connection = arg; - KIRQL OldIrql;
/* Make sure the socket didn't get closed */ if (!arg) return;
- /* Check if data is left to be read */ - LockObject(Connection, &OldIrql); - if (IsListEmpty(&Connection->PacketQueue)) - { - UnlockObject(Connection, OldIrql); - - /* Deliver the error now */ - TCPFinEventHandler(arg, err); - } - else - { - UnlockObject(Connection, OldIrql); - - /* Defer the error delivery until all data is gone */ - Connection->ReceiveShutdown = TRUE; - Connection->ReceiveShutdownStatus = TCPTranslateError(err); - - TCPRecvEventHandler(arg); - } + /* The PCB is dead now */ + Connection->SocketContext = NULL; + + /* Defer the error delivery until all data is gone */ + Connection->ReceiveShutdown = TRUE; + Connection->ReceiveShutdownStatus = TCPTranslateError(err); + + TCPRecvEventHandler(arg); }
static @@ -633,7 +633,12 @@ goto done; }
- /* These need to be called separately, otherwise we get a tcp_close() */ + /* LwIP makes the (questionable) assumption that SHUTDOWN_RDWR is equivalent to tcp_close(). + * This assumption holds even if the shutdown calls are done separately (even through multiple + * WinSock shutdown() calls). This assumption means that lwIP has the right to deallocate our + * PCB without telling us if we shutdown TX and RX. To avoid these problems, we'll clear the + * socket context if we have called shutdown for TX and RX. + */ if (msg->Input.Shutdown.shut_rx) { msg->Output.Shutdown.Error = tcp_shutdown(pcb, TRUE, FALSE); } @@ -651,6 +656,14 @@
if (msg->Input.Shutdown.shut_tx) msg->Input.Shutdown.Connection->SendShutdown = TRUE; + + if (msg->Input.Shutdown.Connection->ReceiveShutdown && + msg->Input.Shutdown.Connection->SendShutdown) + { + /* The PCB is not ours anymore */ + msg->Input.Shutdown.Connection->SocketContext = NULL; + tcp_arg(pcb, NULL); + } }
done: @@ -697,37 +710,43 @@ /* Empty the queue even if we're already "closed" */ LibTCPEmptyQueue(msg->Input.Close.Connection);
- if (!msg->Input.Close.Connection->SocketContext) + /* Check if we've already been closed */ + if (msg->Input.Close.Connection->Closing) { msg->Output.Close.Error = ERR_OK; goto done; }
- /* Clear the PCB pointer */ + /* Enter "closing" mode if we're doing a normal close */ + if (msg->Input.Close.Callback) + msg->Input.Close.Connection->Closing = TRUE; + + /* Check if the PCB was already "closed" but the client doesn't know it yet */ + if (!msg->Input.Close.Connection->SocketContext) + { + if (msg->Input.Close.Callback) + TCPFinEventHandler(msg->Input.Close.Connection, ERR_CLSD); + msg->Output.Close.Error = ERR_OK; + goto done; + } + + /* Clear the PCB pointer and stop callbacks */ msg->Input.Close.Connection->SocketContext = NULL; - - switch (pcb->state) - { - case CLOSED: - case LISTEN: - case SYN_SENT: - msg->Output.Close.Error = tcp_close(pcb); - - if (!msg->Output.Close.Error && msg->Input.Close.Callback) - TCPFinEventHandler(msg->Input.Close.Connection, ERR_CLSD); - break; - - default: - /* Abort the socket */ - tcp_abort(pcb); - msg->Output.Close.Error = ERR_OK; - break; - } + tcp_arg(pcb, NULL); + + /* This may generate additional callbacks but we don't care, + * because they're too inconsistent to rely on */ + msg->Output.Close.Error = tcp_close(pcb);
if (msg->Output.Close.Error) { /* Restore the PCB pointer */ msg->Input.Close.Connection->SocketContext = pcb; + msg->Input.Close.Connection->Closing = FALSE; + } + else if (msg->Input.Close.Callback) + { + TCPFinEventHandler(msg->Input.Close.Connection, ERR_CLSD); }
done:
-
cgutman＠svn.reactos.org