[ros-diffs] [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...
cgutman at svn.reactos.org
cgutman at svn.reactos.org
Fri Aug 5 10:28:11 UTC 2011
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/network/tcpip/tcpip/dispatch.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/network/tcpip/tcpip/fileobjs.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/drivers/ip/transport/tcp/event.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/drivers/ip/transport/tcp/tcp.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/drivers/lwip/src/core/tcp_in.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/drivers/lwip/src/include/rosip.h?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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/drivers/lwip/src/rostcp.c?rev=53083&r1=53082&r2=53083&view=diff
==============================================================================
--- 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);
More information about the Ros-diffs
mailing list