From 30526a51dda7e8db483f22a045f32f3a18eea5c7 Mon Sep 17 00:00:00 2001 From: Jiaxin Wu Date: Tue, 31 May 2016 22:17:27 +0800 Subject: [PATCH] NetworkPkg: HttpDxe response/cancel issue fix Remove timeout check for http body message receive. It should be handled in HttpBootDxe driver for http response unblocking implementation. After timeout removed, the Wrap date should not be freed immediately. Only the TCP CompletionToken in Wrap date is canceled or signaled, the Wrap date could be freed. In addition, Http cancel token is also incorrect. Tcp Cancel should be called to cancel TCP CompletionToken in Wrap date before close it directly. Otherwise, some exception behavior may happened. This patch also refine the coding style for HttpDxe driver. Cc: Fu Siyuan Cc: Ye Ting Cc: Zhang Lubo Cc: Hegde Nagaraj P Cc: Gary Lin Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu Reviewed-by: Gary Lin Reviewed-by: Hegde Nagaraj P Reviewed-by: Ye Ting Tested-by: Gary Lin Tested-by: Hegde Nagaraj P --- NetworkPkg/HttpDxe/HttpImpl.c | 98 ++++++++++++---------------------- NetworkPkg/HttpDxe/HttpProto.c | 78 +++++++++++++++------------ NetworkPkg/HttpDxe/HttpProto.h | 4 +- 3 files changed, 77 insertions(+), 103 deletions(-) diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index 12f22dbbb1..ad194dbe0e 100644 --- a/NetworkPkg/HttpDxe/HttpImpl.c +++ b/NetworkPkg/HttpDxe/HttpImpl.c @@ -462,7 +462,6 @@ EfiHttpRequest ( } } - // // Save the RemotePort and RemoteHost. // @@ -578,13 +577,13 @@ EfiHttpRequest ( return EFI_SUCCESS; Error5: - // - // We would have inserted a TxToken only if Request structure is not NULL. - // Hence check before we do a remove in this error case. - // - if (Request != NULL) { - NetMapRemoveTail (&HttpInstance->TxTokens, NULL); - } + // + // We would have inserted a TxToken only if Request structure is not NULL. + // Hence check before we do a remove in this error case. + // + if (Request != NULL) { + NetMapRemoveTail (&HttpInstance->TxTokens, NULL); + } Error4: if (RequestMsg != NULL) { @@ -606,7 +605,6 @@ Error2: } Error1: - if (HostName != NULL) { FreePool (HostName); } @@ -640,7 +638,6 @@ HttpCancelTokens ( IN VOID *Context ) { - EFI_HTTP_TOKEN *Token; HTTP_TOKEN_WRAP *Wrap; HTTP_PROTOCOL *HttpInstance; @@ -658,42 +655,33 @@ HttpCancelTokens ( Wrap = (HTTP_TOKEN_WRAP *) Item->Value; ASSERT (Wrap != NULL); HttpInstance = Wrap->HttpInstance; - - // - // Free resources. - // - NetMapRemoveItem (Map, Item, NULL); if (!HttpInstance->LocalAddressIsIPv6) { - if (Wrap->TcpWrap.Tx4Token.CompletionToken.Event != NULL) { - gBS->CloseEvent (Wrap->TcpWrap.Tx4Token.CompletionToken.Event); - } - if (Wrap->TcpWrap.Rx4Token.CompletionToken.Event != NULL) { - gBS->CloseEvent (Wrap->TcpWrap.Rx4Token.CompletionToken.Event); - } - - if (Wrap->TcpWrap.Rx4Token.Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) { - FreePool (Wrap->TcpWrap.Rx4Token.Packet.RxData->FragmentTable[0].FragmentBuffer); - } + // + // Cancle the Token before close its Event. + // + HttpInstance->Tcp4->Cancel (HttpInstance->Tcp4, &Wrap->TcpWrap.Rx4Token.CompletionToken); + // + // Dispatch the DPC queued by the NotifyFunction of the canceled token's events. + // + DispatchDpc (); + } } else { - if (Wrap->TcpWrap.Tx6Token.CompletionToken.Event != NULL) { - gBS->CloseEvent (Wrap->TcpWrap.Tx6Token.CompletionToken.Event); - } - if (Wrap->TcpWrap.Rx6Token.CompletionToken.Event != NULL) { - gBS->CloseEvent (Wrap->TcpWrap.Rx6Token.CompletionToken.Event); - } + // + // Cancle the Token before close its Event. + // + HttpInstance->Tcp6->Cancel (HttpInstance->Tcp6, &Wrap->TcpWrap.Rx6Token.CompletionToken); - if (Wrap->TcpWrap.Rx6Token.Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) { - FreePool (Wrap->TcpWrap.Rx6Token.Packet.RxData->FragmentTable[0].FragmentBuffer); + // + // Dispatch the DPC queued by the NotifyFunction of the canceled token's events. + // + DispatchDpc (); } } - - FreePool (Wrap); - // // If only one item is to be cancel, return EFI_ABORTED to stop // iterating the map any more. @@ -1009,6 +997,7 @@ HttpResponseWorker ( // StatusCodeStr = HttpHeaders + AsciiStrLen (HTTP_VERSION_STR) + 1; if (StatusCodeStr == NULL) { + Status = EFI_NOT_READY; goto Error; } @@ -1019,6 +1008,7 @@ HttpResponseWorker ( // Tmp = AsciiStrStr (HttpHeaders, HTTP_CRLF_STR); if (Tmp == NULL) { + Status = EFI_NOT_READY; goto Error; } @@ -1065,6 +1055,7 @@ HttpResponseWorker ( if (SizeofHeaders != 0) { HeaderTmp = AllocateZeroPool (SizeofHeaders); if (HeaderTmp == NULL) { + Status = EFI_OUT_OF_RESOURCES; goto Error2; } @@ -1204,42 +1195,14 @@ HttpResponseWorker ( ASSERT (HttpInstance->MsgParser != NULL); - if (HttpInstance->TimeoutEvent == NULL) { - // - // Create TimeoutEvent for response - // - Status = gBS->CreateEvent ( - EVT_TIMER, - TPL_CALLBACK, - NULL, - NULL, - &HttpInstance->TimeoutEvent - ); - if (EFI_ERROR (Status)) { - goto Error2; - } - } - - // - // Start the timer, and wait Timeout seconds to receive the body packet. - // - Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND); - if (EFI_ERROR (Status)) { - goto Error2; - } - // // We still need receive more data when there is no cache data and MsgParser is not NULL; // - Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance->TimeoutEvent); - - gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0); - + Status = HttpTcpReceiveBody (Wrap, HttpMsg); if (EFI_ERROR (Status)) { goto Error2; } - FreePool (Wrap); return Status; Exit: @@ -1265,6 +1228,11 @@ Error2: } Error: + Item = NetMapFindKey (&Wrap->HttpInstance->RxTokens, Wrap->HttpToken); + if (Item != NULL) { + NetMapRemoveItem (&Wrap->HttpInstance->RxTokens, Item, NULL); + } + HttpTcpTokenCleanup (Wrap); if (HttpHeaders != NULL) { diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c index afa7fe4b35..486b20304f 100644 --- a/NetworkPkg/HttpDxe/HttpProto.c +++ b/NetworkPkg/HttpDxe/HttpProto.c @@ -148,19 +148,41 @@ HttpTcpReceiveNotifyDpc ( if (UsingIpv6) { gBS->CloseEvent (Wrap->TcpWrap.Rx6Token.CompletionToken.Event); + Wrap->TcpWrap.Rx6Token.CompletionToken.Event = NULL; if (EFI_ERROR (Wrap->TcpWrap.Rx6Token.CompletionToken.Status)) { + DEBUG ((EFI_D_ERROR, "HttpTcpReceiveNotifyDpc: %r!\n", Wrap->TcpWrap.Rx6Token.CompletionToken.Status)); Wrap->HttpToken->Status = Wrap->TcpWrap.Rx6Token.CompletionToken.Status; gBS->SignalEvent (Wrap->HttpToken->Event); + + Item = NetMapFindKey (&HttpInstance->RxTokens, Wrap->HttpToken); + if (Item != NULL) { + NetMapRemoveItem (&HttpInstance->RxTokens, Item, NULL); + } + + FreePool (Wrap); + Wrap = NULL; + return ; } } else { gBS->CloseEvent (Wrap->TcpWrap.Rx4Token.CompletionToken.Event); + Wrap->TcpWrap.Rx4Token.CompletionToken.Event = NULL; if (EFI_ERROR (Wrap->TcpWrap.Rx4Token.CompletionToken.Status)) { + DEBUG ((EFI_D_ERROR, "HttpTcpReceiveNotifyDpc: %r!\n", Wrap->TcpWrap.Rx4Token.CompletionToken.Status)); Wrap->HttpToken->Status = Wrap->TcpWrap.Rx4Token.CompletionToken.Status; gBS->SignalEvent (Wrap->HttpToken->Event); + + Item = NetMapFindKey (&HttpInstance->RxTokens, Wrap->HttpToken); + if (Item != NULL) { + NetMapRemoveItem (&HttpInstance->RxTokens, Item, NULL); + } + + FreePool (Wrap); + Wrap = NULL; + return ; } } @@ -232,6 +254,9 @@ HttpTcpReceiveNotifyDpc ( // Check pending RxTokens and receive the HTTP message. // NetMapIterate (&Wrap->HttpInstance->RxTokens, HttpTcpReceive, NULL); + + FreePool (Wrap); + Wrap = NULL; } /** @@ -408,15 +433,15 @@ HttpCreateTcpTxEvent ( Wrap, &TcpWrap->Tx4Token.CompletionToken.Event ); - if (EFI_ERROR (Status)) { - return Status; - } - - TcpWrap->Tx4Data.Push = TRUE; - TcpWrap->Tx4Data.Urgent = FALSE; - TcpWrap->Tx4Data.FragmentCount = 1; - TcpWrap->Tx4Token.Packet.TxData = &Wrap->TcpWrap.Tx4Data; - TcpWrap->Tx4Token.CompletionToken.Status = EFI_NOT_READY; + if (EFI_ERROR (Status)) { + return Status; + } + + TcpWrap->Tx4Data.Push = TRUE; + TcpWrap->Tx4Data.Urgent = FALSE; + TcpWrap->Tx4Data.FragmentCount = 1; + TcpWrap->Tx4Token.Packet.TxData = &Wrap->TcpWrap.Tx4Data; + TcpWrap->Tx4Token.CompletionToken.Status = EFI_NOT_READY; } else { Status = gBS->CreateEvent ( @@ -436,7 +461,6 @@ HttpCreateTcpTxEvent ( TcpWrap->Tx6Token.Packet.TxData = &Wrap->TcpWrap.Tx6Data; TcpWrap->Tx6Token.CompletionToken.Status =EFI_NOT_READY; - } return EFI_SUCCESS; @@ -1607,6 +1631,10 @@ HttpTcpReceiveHeader ( } if (!HttpInstance->IsRxDone) { + // + // Cancle the Token before close its Event. + // + Tcp4->Cancel (HttpInstance->Tcp4, &Rx4Token->CompletionToken); gBS->CloseEvent (Rx4Token->CompletionToken.Event); Rx4Token->CompletionToken.Status = EFI_TIMEOUT; } @@ -1673,6 +1701,10 @@ HttpTcpReceiveHeader ( } if (!HttpInstance->IsRxDone) { + // + // Cancle the Token before close its Event. + // + Tcp6->Cancel (HttpInstance->Tcp6, &Rx6Token->CompletionToken); gBS->CloseEvent (Rx6Token->CompletionToken.Event); Rx6Token->CompletionToken.Status = EFI_TIMEOUT; } @@ -1728,7 +1760,6 @@ HttpTcpReceiveHeader ( @param[in] Wrap The HTTP token's wrap data. @param[in] HttpMsg The HTTP message data. - @param[in] Timeout The time to wait for receiving the body packet. @retval EFI_SUCCESS The HTTP body is received. @retval Others Other error as indicated. @@ -1737,8 +1768,7 @@ HttpTcpReceiveHeader ( EFI_STATUS HttpTcpReceiveBody ( IN HTTP_TOKEN_WRAP *Wrap, - IN EFI_HTTP_MESSAGE *HttpMsg, - IN EFI_EVENT Timeout + IN EFI_HTTP_MESSAGE *HttpMsg ) { EFI_STATUS Status; @@ -1772,17 +1802,6 @@ HttpTcpReceiveBody ( DEBUG ((EFI_D_ERROR, "Tcp6 receive failed: %r\n", Status)); return Status; } - - while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR (gBS->CheckEvent (Timeout)))) { - Tcp6->Poll (Tcp6); - } - - if (!Wrap->TcpWrap.IsRxDone) { - gBS->CloseEvent (Rx6Token->CompletionToken.Event); - Rx6Token->CompletionToken.Status = EFI_TIMEOUT; - Wrap->HttpToken->Status = Rx6Token->CompletionToken.Status; - gBS->SignalEvent (Wrap->HttpToken->Event); - } } else { Rx4Token = &Wrap->TcpWrap.Rx4Token; Rx4Token->Packet.RxData->DataLength = (UINT32) HttpMsg->BodyLength; @@ -1795,17 +1814,6 @@ HttpTcpReceiveBody ( DEBUG ((EFI_D_ERROR, "Tcp4 receive failed: %r\n", Status)); return Status; } - - while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR (gBS->CheckEvent (Timeout)))) { - Tcp4->Poll (Tcp4); - } - - if (!Wrap->TcpWrap.IsRxDone) { - gBS->CloseEvent (Rx4Token->CompletionToken.Event); - Rx4Token->CompletionToken.Status = EFI_TIMEOUT; - Wrap->HttpToken->Status = Rx4Token->CompletionToken.Status; - gBS->SignalEvent (Wrap->HttpToken->Event); - } } return EFI_SUCCESS; diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h index cdad5b0e48..e1fd785b2c 100644 --- a/NetworkPkg/HttpDxe/HttpProto.h +++ b/NetworkPkg/HttpDxe/HttpProto.h @@ -525,7 +525,6 @@ HttpTcpReceiveHeader ( @param[in] Wrap The HTTP token's wrap data. @param[in] HttpMsg The HTTP message data. - @param[in] Timeout The time to wait for receiving the body packet. @retval EFI_SUCCESS The HTTP body is received. @retval Others Other error as indicated. @@ -534,8 +533,7 @@ HttpTcpReceiveHeader ( EFI_STATUS HttpTcpReceiveBody ( IN HTTP_TOKEN_WRAP *Wrap, - IN EFI_HTTP_MESSAGE *HttpMsg, - IN EFI_EVENT Timeout + IN EFI_HTTP_MESSAGE *HttpMsg ); /**