diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp index 29a7088e4c4819..d5651cab85d0ff 100644 --- a/src/inet/TCPEndPoint.cpp +++ b/src/inet/TCPEndPoint.cpp @@ -322,10 +322,8 @@ CHIP_ERROR TCPEndPoint::DriveSending() return err; } -void TCPEndPoint::DriveReceiving() +void TCPEndPoint::DriveReceiving(const TCPEndPointHandle & handle) { - TCPEndPointHandle handle(this); - // If there's data in the receive queue and the app is ready to receive it then call the app's callback // with the entire receive queue. if (!mRcvQueue.IsNull() && mReceiveEnabled && OnDataReceived != nullptr) @@ -438,7 +436,8 @@ void TCPEndPoint::DoClose(CHIP_ERROR err, bool suppressCallback) oldState == State::kClosing) && OnConnectionClosed != nullptr) { - OnConnectionClosed(*this, err); + TCPEndPointHandle handle(this); + OnConnectionClosed(handle, err); } } } diff --git a/src/inet/TCPEndPoint.h b/src/inet/TCPEndPoint.h index 6269a5a83fb8f4..1947643e234057 100644 --- a/src/inet/TCPEndPoint.h +++ b/src/inet/TCPEndPoint.h @@ -199,7 +199,8 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis void EnableReceive() { mReceiveEnabled = true; - DriveReceiving(); + TCPEndPointHandle handle(this); + DriveReceiving(handle); } /** @@ -455,7 +456,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis * member to process connection termination events on \c endPoint. The * \c err argument distinguishes successful terminations from failures. */ - typedef void (*OnConnectionClosedFunct)(TCPEndPoint & endPoint, CHIP_ERROR err); + typedef void (*OnConnectionClosedFunct)(const TCPEndPointHandle & endPoint, CHIP_ERROR err); /** The endpoint's close event handling function delegate. */ OnConnectionClosedFunct OnConnectionClosed; @@ -469,7 +470,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis * Provide a function of this type to the \c OnPeerClose delegate member * to process connection termination events on \c endPoint. */ - typedef void (*OnPeerCloseFunct)(TCPEndPoint & endPoint); + typedef void (*OnPeerCloseFunct)(const TCPEndPointHandle & endPoint); /** The endpoint's half-close receive event handling function delegate. */ OnPeerCloseFunct OnPeerClose; @@ -497,15 +498,15 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis /** * @brief Type of connection acceptance error event handling function. * - * @param[in] endPoint The TCP endpoint associated with the event. - * @param[in] err The reason for the error. + * @param[in] listeningEndpoint The listening TCP endpoint associated with the event. + * @param[in] err The reason for the error. * * @details * Provide a function of this type to the \c OnAcceptError delegate - * member to process connection acceptance error events on \c endPoint. The + * member to process connection acceptance error events on \c listeningEndpoint. The * \c err argument provides specific detail about the type of the error. */ - typedef void (*OnAcceptErrorFunct)(const TCPEndPointHandle & endPoint, CHIP_ERROR err); + typedef void (*OnAcceptErrorFunct)(const TCPEndPointHandle & listeningEndpoint, CHIP_ERROR err); /** * The endpoint's connection acceptance event handling function delegate. @@ -593,7 +594,8 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis TCPEndPoint(const TCPEndPoint &) = delete; CHIP_ERROR DriveSending(); - void DriveReceiving(); + // Take in a handle to self to guarantee this sticks around until we're done + void DriveReceiving(const TCPEndPointHandle & handle); void HandleConnectComplete(CHIP_ERROR err); void HandleAcceptError(CHIP_ERROR err); void DoClose(CHIP_ERROR err, bool suppressCallback); diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 030e56d7ded4ee..27f07b79714e1c 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -653,53 +653,56 @@ void TCPEndPointImplLwIP::HandleDataSent(uint16_t lenSent) void TCPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && buf) { // Only receive new data while in the Connected or SendShutdown states. - if (mState == State::kConnected || mState == State::kSendShutdown) + if (mState != State::kConnected && mState != State::kSendShutdown) { - // Mark the connection as being active. - MarkActive(); + return; + } + + TCPEndPointHandle handle(this); + // Mark the connection as being active. + MarkActive(); - // If we received a data buffer, queue it on the receive queue. If there's already data in - // the queue, compact the data into the head buffer. - if (!buf.IsNull()) + // If we received a data buffer, queue it on the receive queue. If there's already data in + // the queue, compact the data into the head buffer. + if (!buf.IsNull()) + { + if (mRcvQueue.IsNull()) { - if (mRcvQueue.IsNull()) - { - mRcvQueue = std::move(buf); - } - else - { - mRcvQueue->AddToEnd(std::move(buf)); - mRcvQueue->CompactHead(); - } + mRcvQueue = std::move(buf); } - - // Otherwise buf == NULL means the other side closed the connection, so ... else { - - // If in the Connected state and the app has provided an OnPeerClose callback, - // enter the ReceiveShutdown state. Providing an OnPeerClose callback allows - // the app to decide whether to keep the send side of the connection open after - // the peer has closed. If no OnPeerClose is provided, we assume that the app - // wants to close both directions and automatically enter the Closing state. - if (mState == State::kConnected && OnPeerClose != nullptr) - { - mState = State::kReceiveShutdown; - } - else - { - mState = State::kClosing; - } - // Call the app's OnPeerClose. - if (OnPeerClose != NULL) - { - OnPeerClose(*this); - } + mRcvQueue->AddToEnd(std::move(buf)); + mRcvQueue->CompactHead(); } + } - // Drive the received data into the app. - DriveReceiving(); + // Otherwise buf == NULL means the other side closed the connection, so ... + else + { + + // If in the Connected state and the app has provided an OnPeerClose callback, + // enter the ReceiveShutdown state. Providing an OnPeerClose callback allows + // the app to decide whether to keep the send side of the connection open after + // the peer has closed. If no OnPeerClose is provided, we assume that the app + // wants to close both directions and automatically enter the Closing state. + if (mState == State::kConnected && OnPeerClose != nullptr) + { + mState = State::kReceiveShutdown; + } + else + { + mState = State::kClosing; + } + // Call the app's OnPeerClose. + if (OnPeerClose != NULL) + { + OnPeerClose(handle); + } } + + // Drive the received data into the app. + DriveReceiving(handle); } void TCPEndPointImplLwIP::HandleIncomingConnection(const TCPEndPointHandle & conEP) diff --git a/src/inet/TCPEndPointImplSockets.cpp b/src/inet/TCPEndPointImplSockets.cpp index d803d375066f88..0d3ccdf9b98203 100644 --- a/src/inet/TCPEndPointImplSockets.cpp +++ b/src/inet/TCPEndPointImplSockets.cpp @@ -904,6 +904,7 @@ void TCPEndPointImplSockets::ReceiveData() RestartTCPUserTimeoutTimer(); } #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT + TCPEndPointHandle handle(this); // If an error occurred, abort the connection. if (rcvLen < 0) { @@ -921,7 +922,6 @@ void TCPEndPointImplSockets::ReceiveData() DoClose(CHIP_ERROR_POSIX(systemErrno), false); } - else { // Mark the connection as being active. @@ -948,7 +948,7 @@ void TCPEndPointImplSockets::ReceiveData() // Call the app's OnPeerClose. if (OnPeerClose != nullptr) { - OnPeerClose(*this); + OnPeerClose(handle); } } @@ -978,7 +978,7 @@ void TCPEndPointImplSockets::ReceiveData() } // Drive any received data into the app. - DriveReceiving(); + DriveReceiving(handle); } CHIP_ERROR TCPEndPointImplSockets::HandleIncomingConnection() diff --git a/src/inet/tests/inet-layer-test-tool.cpp b/src/inet/tests/inet-layer-test-tool.cpp index e46814e05ce992..13de5c4783f7c0 100644 --- a/src/inet/tests/inet-layer-test-tool.cpp +++ b/src/inet/tests/inet-layer-test-tool.cpp @@ -547,7 +547,7 @@ void HandleTCPConnectionComplete(const TCPEndPointHandle & aEndPoint, CHIP_ERROR } } -static void HandleTCPConnectionClosed(TCPEndPoint & aEndPoint, CHIP_ERROR aError) +static void HandleTCPConnectionClosed(const TCPEndPointHandle & aEndPoint, CHIP_ERROR aError) { if (aError == CHIP_NO_ERROR) { diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp index 21aa920fbae64c..80a25aa6012fd1 100644 --- a/src/transport/raw/TCP.cpp +++ b/src/transport/raw/TCP.cpp @@ -594,10 +594,10 @@ void TCPBase::HandleTCPEndPointConnectComplete(const Inet::TCPEndPointHandle & e tcp->HandleConnectionAttemptComplete(activeConnection, CHIP_NO_ERROR); } -void TCPBase::HandleTCPEndPointConnectionClosed(Inet::TCPEndPoint & endPoint, CHIP_ERROR err) +void TCPBase::HandleTCPEndPointConnectionClosed(const Inet::TCPEndPointHandle & endPoint, CHIP_ERROR err) { - TCPBase * tcp = reinterpret_cast(endPoint.mAppState); - ActiveTCPConnectionHandle activeConnection = tcp->FindInUseConnection(endPoint); + TCPBase * tcp = reinterpret_cast(endPoint->mAppState); + ActiveTCPConnectionHandle activeConnection = tcp->FindInUseConnection(*endPoint); if (activeConnection.IsNull()) { @@ -617,56 +617,53 @@ void TCPBase::HandleIncomingConnection(const Inet::TCPEndPointHandle & listenEnd const Inet::IPAddress & peerAddress, uint16_t peerPort) { TCPBase * tcp = reinterpret_cast(listenEndPoint->mAppState); - // To handle errors around endpoint in the calls to HandleAcceptError, we must initialize mAppState - endPoint->mAppState = tcp; + ReturnAndLogOnFailure(tcp->DoHandleIncomingConnection(listenEndPoint, endPoint, peerAddress, peerPort), Inet, + "Failure accepting incoming connection"); +} +CHIP_ERROR TCPBase::DoHandleIncomingConnection(const Inet::TCPEndPointHandle & listenEndPoint, + const Inet::TCPEndPointHandle & endPoint, const Inet::IPAddress & peerAddress, + uint16_t peerPort) +{ PeerAddress addr; - CHIP_ERROR err = GetPeerAddress(*endPoint, addr); - VerifyOrReturn(err == CHIP_NO_ERROR, HandleAcceptError(endPoint, err)); - ActiveTCPConnectionState * activeConnection = tcp->AllocateConnection(endPoint, addr); - if (activeConnection != nullptr) + ReturnErrorOnFailure(GetPeerAddress(*endPoint, addr)); + ActiveTCPConnectionState * activeConnection = AllocateConnection(endPoint, addr); + if (activeConnection == nullptr) { - auto connectionCleanup = ScopeExit([&]() { activeConnection->Free(); }); + return CHIP_ERROR_TOO_MANY_CONNECTIONS; + } + + auto connectionCleanup = ScopeExit([&]() { activeConnection->Free(); }); + + endPoint->mAppState = this; + endPoint->OnDataReceived = HandleTCPEndPointDataReceived; + endPoint->OnDataSent = nullptr; + endPoint->OnConnectionClosed = HandleTCPEndPointConnectionClosed; - endPoint->OnDataReceived = HandleTCPEndPointDataReceived; - endPoint->OnDataSent = nullptr; - endPoint->OnConnectionClosed = HandleTCPEndPointConnectionClosed; + // By default, disable TCP Nagle buffering by setting TCP_NODELAY socket option to true + endPoint->EnableNoDelay(); - // By default, disable TCP Nagle buffering by setting TCP_NODELAY socket option to true - endPoint->EnableNoDelay(); + mUsedEndPointCount++; + activeConnection->mConnectionState = TCPState::kConnected; - tcp->mUsedEndPointCount++; - activeConnection->mConnectionState = TCPState::kConnected; + // Set the TCPKeepalive configurations on the received connection + endPoint->EnableKeepAlive(activeConnection->mTCPKeepAliveIntervalSecs, activeConnection->mTCPMaxNumKeepAliveProbes); - // Set the TCPKeepalive configurations on the received connection - endPoint->EnableKeepAlive(activeConnection->mTCPKeepAliveIntervalSecs, activeConnection->mTCPMaxNumKeepAliveProbes); + char addrStr[Transport::PeerAddress::kMaxToStringSize]; + peerAddress.ToString(addrStr); + ChipLogProgress(Inet, "Incoming connection established with peer at %s.", addrStr); - char addrStr[Transport::PeerAddress::kMaxToStringSize]; - peerAddress.ToString(addrStr); - ChipLogProgress(Inet, "Incoming connection established with peer at %s.", addrStr); + // Call the upper layer handler for incoming connection received. + HandleConnectionReceived(*activeConnection); - // Call the upper layer handler for incoming connection received. - tcp->HandleConnectionReceived(*activeConnection); + connectionCleanup.release(); - connectionCleanup.release(); - } - else - { - ChipLogError(Inet, "Insufficient connection space to accept new connections."); - HandleAcceptError(endPoint, CHIP_ERROR_TOO_MANY_CONNECTIONS); - } + return CHIP_NO_ERROR; } -void TCPBase::HandleAcceptError(const Inet::TCPEndPointHandle & endPoint, CHIP_ERROR err) +void TCPBase::HandleAcceptError(const Inet::TCPEndPointHandle & listeningEndpoint, CHIP_ERROR err) { ChipLogError(Inet, "Accept error: %" CHIP_ERROR_FORMAT, err.Format()); - TCPBase * tcp = reinterpret_cast(endPoint->mAppState); - - ActiveTCPConnectionState * conn = tcp->FindActiveConnection(endPoint); - if (conn != nullptr) - { - tcp->CloseConnectionInternal(*conn, err, SuppressCallback::No); - } } CHIP_ERROR TCPBase::TCPConnect(const PeerAddress & address, Transport::AppTCPConnectionCallbackCtxt * appState, diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h index 15879b61a41bcc..5e9a165fc90c64 100644 --- a/src/transport/raw/TCP.h +++ b/src/transport/raw/TCP.h @@ -290,13 +290,16 @@ class DLL_EXPORT TCPBase : public Base // Callback handler for TCPEndPoint. Called when a connection has been closed. // @see TCPEndpoint::OnConnectionClosedFunct - static void HandleTCPEndPointConnectionClosed(Inet::TCPEndPoint & endPoint, CHIP_ERROR err); + static void HandleTCPEndPointConnectionClosed(const Inet::TCPEndPointHandle & endPoint, CHIP_ERROR err); // Callback handler for TCPEndPoint. Called when a connection is received on the listening port. // @see TCPEndpoint::OnConnectionReceivedFunct static void HandleIncomingConnection(const Inet::TCPEndPointHandle & listenEndPoint, const Inet::TCPEndPointHandle & endPoint, const Inet::IPAddress & peerAddress, uint16_t peerPort); + CHIP_ERROR DoHandleIncomingConnection(const Inet::TCPEndPointHandle & listenEndPoint, const Inet::TCPEndPointHandle & endPoint, + const Inet::IPAddress & peerAddress, uint16_t peerPort); + // Callback handler for handling accept error // @see TCPEndpoint::OnAcceptErrorFunct static void HandleAcceptError(const Inet::TCPEndPointHandle & endPoint, CHIP_ERROR err);