[cgutman] 53083: [TCPIP] - Fix socket closure from a hacked mess that just aborts every socket into something that works properly [LWIP] - Add code to indicate when a socket is closed from LAST_ACK...
Author: cgutman Date: Fri Aug 5 10:28:11 2011 New Revision: 53083
URL: http://svn.reactos.org/svn/reactos?rev=53083&view=rev Log: [TCPIP] - Fix socket closure from a hacked mess that just aborts every socket into something that works properly [LWIP] - Add code to indicate when a socket is closed from LAST_ACK state - The problem was that after we call tcp_close from a CLOSE_WAIT state, we never get any indication when the connection is actually closed which causes problems because we're stuck guessing when we can release the port - I will bring this issue up on the lwIP mailing lists to see if this is a patch they want to integrate or if they have any other ideas
Modified: branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/dispatch.c branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/fileobjs.c branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/event.c branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/tcp.c branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/core/tcp_in.c branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/include/rosip.h branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/rostcp.c
Modified: branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/dispatch.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/drivers/ne... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/dispatch.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/dispatch.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -444,11 +444,9 @@ * Status of operation */ { - PCONNECTION_ENDPOINT Connection, LastConnection; + PCONNECTION_ENDPOINT Connection; PTRANSPORT_CONTEXT TranContext; PIO_STACK_LOCATION IrpSp; - KIRQL OldIrql; - NTSTATUS Status;
TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
@@ -468,52 +466,9 @@ return STATUS_INVALID_PARAMETER; }
- LockObject(Connection, &OldIrql); - - if (!Connection->AddressFile) { - UnlockObject(Connection, OldIrql); - TI_DbgPrint(MID_TRACE, ("No address file is asscociated.\n")); - return STATUS_INVALID_PARAMETER; - } - - LockObjectAtDpcLevel(Connection->AddressFile); - - /* Unlink this connection from the address file */ - if (Connection->AddressFile->Connection == Connection) - { - Connection->AddressFile->Connection = Connection->Next; - DereferenceObject(Connection); - Status = STATUS_SUCCESS; - } - else - { - LastConnection = Connection->AddressFile->Connection; - while (LastConnection->Next != Connection && LastConnection->Next != NULL) - LastConnection = LastConnection->Next; - if (LastConnection->Next == Connection) - { - LastConnection->Next = Connection->Next; - DereferenceObject(Connection); - Status = STATUS_SUCCESS; - } - else - { - Status = STATUS_INVALID_PARAMETER; - } - } - - UnlockObjectFromDpcLevel(Connection->AddressFile); - - if (Status == STATUS_SUCCESS) - { - /* Remove the address file from this connection */ - DereferenceObject(Connection->AddressFile); - Connection->AddressFile = NULL; - } - - UnlockObject(Connection, OldIrql); - - return Status; + /* NO-OP because we need the address to deallocate the port when the connection closes */ + + return STATUS_SUCCESS; }
@@ -653,6 +608,7 @@ Status = STATUS_NO_MEMORY;
if( NT_SUCCESS(Status) ) { + ReferenceObject(Connection->AddressFile); Connection->AddressFile->Listener->AddressFile = Connection->AddressFile;
Modified: branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/fileobjs.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/drivers/ne... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -409,7 +409,6 @@ /* We have to close this listener because we started it */ if( AddrFile->Listener ) { - AddrFile->Listener->AddressFile = NULL; TCPClose( AddrFile->Listener ); }
Modified: branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/event.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/lib/driver... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/event.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/event.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -63,7 +63,7 @@ }
VOID -FlushAllQueues(PCONNECTION_ENDPOINT Connection, const NTSTATUS Status) +FlushReceiveQueue(PCONNECTION_ENDPOINT Connection, const NTSTATUS Status) { PTDI_BUCKET Bucket; PLIST_ENTRY Entry; @@ -84,13 +84,35 @@ CompleteBucket(Connection, Bucket, FALSE); }
- // Calling with Status == STATUS_SUCCESS means that we got a graceful closure - // so we don't want to kill everything else since send is still valid in this state - // + DereferenceObject(Connection); +} + +VOID +FlushAllQueues(PCONNECTION_ENDPOINT Connection, NTSTATUS Status) +{ + PTDI_BUCKET Bucket; + PLIST_ENTRY Entry; + + ReferenceObject(Connection); + + while ((Entry = ExInterlockedRemoveHeadList(&Connection->ReceiveRequest, &Connection->Lock))) + { + Bucket = CONTAINING_RECORD( Entry, TDI_BUCKET, Entry ); + + TI_DbgPrint(DEBUG_TCP, + ("Completing Receive request: %x %x\n", + Bucket->Request, Status)); + + Bucket->Status = Status; + Bucket->Information = 0; + + CompleteBucket(Connection, Bucket, FALSE); + } + + /* We completed the reads successfully but we need to return failure now */ if (Status == STATUS_SUCCESS) { - DereferenceObject(Connection); - return; + Status = STATUS_FILE_CLOSED; }
while ((Entry = ExInterlockedRemoveHeadList(&Connection->ListenRequest, &Connection->Lock))) @@ -134,17 +156,61 @@ VOID TCPFinEventHandler(void *arg, const err_t err) { - PCONNECTION_ENDPOINT Connection = (PCONNECTION_ENDPOINT)arg; - const NTSTATUS status = TCPTranslateError(err); - - /* Only clear the pointer if the shutdown was caused by an error */ - if (err != ERR_OK) - { - /* We're got closed by the error so remove the PCB pointer */ + PCONNECTION_ENDPOINT Connection = (PCONNECTION_ENDPOINT)arg, LastConnection; + const NTSTATUS Status = TCPTranslateError(err); + KIRQL OldIrql; + + ASSERT(Connection->AddressFile); + + /* Check if this was a partial socket closure */ + if (err == ERR_OK && Connection->SocketContext) + { + /* Just flush the receive queue and get out of here */ + FlushReceiveQueue(Connection, STATUS_SUCCESS); + } + else + { + /* First off all, remove the PCB pointer */ Connection->SocketContext = NULL; - } - - FlushAllQueues(Connection, status); + + /* Complete all outstanding requests now */ + FlushAllQueues(Connection, Status); + + LockObject(Connection, &OldIrql); + + LockObjectAtDpcLevel(Connection->AddressFile); + + /* Unlink this connection from the address file */ + if (Connection->AddressFile->Connection == Connection) + { + Connection->AddressFile->Connection = Connection->Next; + DereferenceObject(Connection); + } + else if (Connection->AddressFile->Listener == Connection) + { + Connection->AddressFile->Listener = NULL; + DereferenceObject(Connection); + } + else + { + LastConnection = Connection->AddressFile->Connection; + while (LastConnection->Next != Connection && LastConnection->Next != NULL) + LastConnection = LastConnection->Next; + if (LastConnection->Next == Connection) + { + LastConnection->Next = Connection->Next; + DereferenceObject(Connection); + } + } + + UnlockObjectFromDpcLevel(Connection->AddressFile); + + /* Remove the address file from this connection */ + DereferenceObject(Connection->AddressFile); + Connection->AddressFile = NULL; + + UnlockObject(Connection, OldIrql); + } }
VOID @@ -186,7 +252,7 @@ ASSERT( ((PTCP_PCB)Bucket->AssociatedEndpoint->SocketContext)->state == CLOSED );
/* free socket context created in FileOpenConnection, as we're using a new one */ - LibTCPClose(Bucket->AssociatedEndpoint, TRUE); + LibTCPClose(Bucket->AssociatedEndpoint, TRUE, FALSE);
/* free previously created socket context (we don't use it, we use newpcb) */ Bucket->AssociatedEndpoint->SocketContext = newpcb;
Modified: branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/tcp.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/lib/driver... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -106,15 +106,12 @@
Socket = Connection->SocketContext;
- /* We should not be associated to an address file at this point */ - ASSERT(!Connection->AddressFile); - /* Don't try to close again if the other side closed us already */ if (Connection->SocketContext) { FlushAllQueues(Connection, STATUS_CANCELLED);
- LibTCPClose(Connection, FALSE); + LibTCPClose(Connection, FALSE, TRUE); }
UnlockObject(Connection, OldIrql);
Modified: branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/core/tcp_in.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/lib/driver... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/core/tcp_in.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/core/tcp_in.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -345,6 +345,10 @@ } else if (recv_flags & TF_CLOSED) { /* The connection has been closed and we will deallocate the PCB. */ + TCP_EVENT_CLOSED(pcb, err); + if (err == ERR_ABRT) { + goto aborted; + } tcp_pcb_remove(&tcp_active_pcbs, pcb); memp_free(MEMP_TCP_PCB, pcb); } else {
Modified: branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/include/rosip.h URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/lib/driver... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/include/rosip.h [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/include/rosip.h [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -34,7 +34,7 @@ err_t LibTCPSend(PCONNECTION_ENDPOINT Connection, void *const dataptr, const u16_t len, const int safe); err_t LibTCPConnect(PCONNECTION_ENDPOINT Connection, struct ip_addr *const ipaddr, const u16_t port); err_t LibTCPShutdown(PCONNECTION_ENDPOINT Connection, const int shut_rx, const int shut_tx); -err_t LibTCPClose(PCONNECTION_ENDPOINT Connection, const int safe); +err_t LibTCPClose(PCONNECTION_ENDPOINT Connection, const int safe, const int callback);
err_t LibTCPGetPeerName(PTCP_PCB pcb, struct ip_addr *const ipaddr, u16_t *const port); err_t LibTCPGetHostName(PTCP_PCB pcb, struct ip_addr *const ipaddr, u16_t *const port);
Modified: branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/rostcp.c URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2011/TcpIpDriver/lib/driver... ============================================================================== --- branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/rostcp.c [iso-8859-1] (original) +++ branches/GSoC_2011/TcpIpDriver/lib/drivers/lwip/src/rostcp.c [iso-8859-1] Fri Aug 5 10:28:11 2011 @@ -599,27 +599,25 @@ LibTCPShutdownCallback(void *arg) { struct shutdown_callback_msg *msg = arg; - + PTCP_PCB pcb = msg->Connection->SocketContext; + if (!msg->Connection->SocketContext) { msg->Error = ERR_CLSD; goto done; }
- /* - We check here if the pcb is in state ESTABLISHED or SYN_RECV because otherwise - it means lwIP will take care of it anyway and if it does so before us it will - cause memory corruption. - */ - if ((((PTCP_PCB)msg->Connection->SocketContext)->state == ESTABLISHED) || - (((PTCP_PCB)msg->Connection->SocketContext)->state == SYN_RCVD)) - { - msg->Error = - tcp_shutdown((PTCP_PCB)msg->Connection->SocketContext, - msg->shut_rx, msg->shut_tx); - } - else - msg->Error = ERR_OK; + if (pcb->state == CLOSE_WAIT) + { + /* This case actually results in a socket closure later (lwIP bug?) */ + msg->Connection->SocketContext = NULL; + } + + msg->Error = tcp_shutdown(pcb, msg->shut_rx, msg->shut_tx); + if (msg->Error) + { + msg->Connection->SocketContext = pcb; + }
done: KeSetEvent(&msg->Event, IO_NO_INCREMENT, FALSE); @@ -662,6 +660,7 @@
/* Input */ PCONNECTION_ENDPOINT Connection; + int Callback;
/* Output */ err_t Error; @@ -672,6 +671,8 @@ LibTCPCloseCallback(void *arg) { struct close_callback_msg *msg = arg; + PTCP_PCB pcb = msg->Connection->SocketContext; + int state;
if (!msg->Connection->SocketContext) { @@ -681,22 +682,43 @@
LibTCPEmptyQueue(msg->Connection);
- if (((PTCP_PCB)msg->Connection->SocketContext)->state == LISTEN) - { - msg->Error = tcp_close((PTCP_PCB)msg->Connection->SocketContext); + /* Clear the PCB pointer */ + msg->Connection->SocketContext = NULL; + + /* Save the old PCB state */ + state = pcb->state; + + msg->Error = tcp_close(pcb); + if (!msg->Error) + { + if (msg->Callback) + { + /* Call the FIN handler in the cases where it will not be called by lwIP */ + switch (state) + { + case CLOSED: + case LISTEN: + case SYN_SENT: + TCPFinEventHandler(msg->Connection, ERR_OK); + break; + + default: + break; + } + } } else { - tcp_abort((PTCP_PCB)msg->Connection->SocketContext); - msg->Error = ERR_OK; - } - + /* Restore the PCB pointer */ + msg->Connection->SocketContext = pcb; + } + done: KeSetEvent(&msg->Event, IO_NO_INCREMENT, FALSE); }
err_t -LibTCPClose(PCONNECTION_ENDPOINT Connection, const int safe) +LibTCPClose(PCONNECTION_ENDPOINT Connection, const int safe, const int callback) { err_t ret; struct close_callback_msg *msg; @@ -707,6 +729,7 @@ KeInitializeEvent(&msg->Event, NotificationEvent, FALSE);
msg->Connection = Connection; + msg->Callback = callback;
if (safe) LibTCPCloseCallback(msg);
-
cgutman＠svn.reactos.org