From f3787bdef416c48f4d1906d8745fbeb9781fa376 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 31 Dec 2024 17:24:41 +0800 Subject: [PATCH 01/29] TCP: Use RunOnTCPIP for the LwIP TCP EndPoint --- src/inet/TCPEndPointImplLwIP.cpp | 311 +++++++++++--------------- src/platform/PlatformEventSupport.cpp | 10 + src/system/PlatformEventSupport.h | 2 + src/system/SystemLayer.cpp | 9 + src/system/SystemLayer.h | 3 + 5 files changed, 158 insertions(+), 177 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 763f2eb9613078..2d991fcf295565 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -39,9 +39,6 @@ static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later"); -// TODO: Update to use RunOnTCPIP. -static_assert(LWIP_TCPIP_CORE_LOCKING, "CHIP requires config LWIP_TCPIP_CORE_LOCKING enabled"); - namespace chip { namespace Inet { @@ -58,16 +55,13 @@ void nil_tcpip_callback(void * _aContext) {} err_t start_tcp_timers(void) { - return tcpip_callback(nil_tcpip_callback, NULL); + return tcpip_callback(nil_tcpip_callback, nullptr); } } // anonymous namespace CHIP_ERROR TCPEndPointImplLwIP::BindImpl(IPAddressType addrType, const IPAddress & addr, uint16_t port, bool reuseAddr) { - // Lock LwIP stack - LOCK_TCPIP_CORE(); - // Get the appropriate type of PCB. CHIP_ERROR res = GetPCB(addrType); @@ -77,31 +71,28 @@ CHIP_ERROR TCPEndPointImplLwIP::BindImpl(IPAddressType addrType, const IPAddress { if (reuseAddr) { - ip_set_option(mTCP, SOF_REUSEADDR); + RunOnTCPIP([this]() { ip_set_option(mTCP, SOF_REUSEADDR); }); } res = addr.ToLwIPAddr(addrType, ipAddr); } if (res == CHIP_NO_ERROR) { - res = chip::System::MapErrorLwIP(tcp_bind(mTCP, &ipAddr, port)); + res = chip::System::MapErrorLwIP(RunOnTCPIPRet([this, &ipAddr, port]() { return tcp_bind(mTCP, &ipAddr, port); })); } - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); return res; } CHIP_ERROR TCPEndPointImplLwIP::ListenImpl(uint16_t backlog) { - // Start listening for incoming connections. - mTCP = tcp_listen(mTCP); mLwIPEndPointType = LwIPEndPointType::TCP; - - tcp_arg(mTCP, this); - - tcp_accept(mTCP, LwIPHandleIncomingConnection); - + RunOnTCPIP([this]() { + // Start listening for incoming connections. + mTCP = tcp_listen(mTCP); + tcp_arg(mTCP, this); + tcp_accept(mTCP, LwIPHandleIncomingConnection); + }); return CHIP_NO_ERROR; } @@ -130,18 +121,16 @@ CHIP_ERROR TCPEndPointImplLwIP::ConnectImpl(const IPAddress & addr, uint16_t por return res; } - // Lock LwIP stack - LOCK_TCPIP_CORE(); - res = GetPCB(addrType); if (res == CHIP_NO_ERROR) { - tcp_arg(mTCP, this); - tcp_err(mTCP, LwIPHandleError); - ip_addr_t lwipAddr = addr.ToLwIPAddr(); - res = chip::System::MapErrorLwIP(tcp_connect(mTCP, &lwipAddr, port, LwIPHandleConnectComplete)); + res = chip::System::MapErrorLwIP(RunOnTCPIPRet([this, &lwipAddr, port]() { + tcp_arg(mTCP, this); + tcp_err(mTCP, LwIPHandleError); + return tcp_connect(mTCP, &lwipAddr, port, LwIPHandleConnectComplete); + })); // Ensure that TCP timers are started if (res == CHIP_NO_ERROR) @@ -160,8 +149,6 @@ CHIP_ERROR TCPEndPointImplLwIP::ConnectImpl(const IPAddress & addr, uint16_t por } } - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); return res; } @@ -169,42 +156,30 @@ CHIP_ERROR TCPEndPointImplLwIP::GetPeerInfo(IPAddress * retAddr, uint16_t * retP { VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE); - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED; if (mTCP != nullptr) { - *retPort = mTCP->remote_port; - *retAddr = IPAddress(mTCP->remote_ip); - res = CHIP_NO_ERROR; + RunOnTCPIP([this, &retAddr, &retPort]() { + *retPort = mTCP->remote_port; + *retAddr = IPAddress(mTCP->remote_ip); + }); + return CHIP_NO_ERROR; } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - - return res; + return CHIP_ERROR_CONNECTION_ABORTED; } CHIP_ERROR TCPEndPointImplLwIP::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort) const { VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE); - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED; if (mTCP != nullptr) { - *retPort = mTCP->local_port; - *retAddr = IPAddress(mTCP->local_ip); - res = CHIP_NO_ERROR; + RunOnTCPIP([this, &retAddr, &retPort]() { + *retPort = mTCP->local_port; + *retAddr = IPAddress(mTCP->local_ip); + }); + return CHIP_NO_ERROR; } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - - return res; + return CHIP_ERROR_CONNECTION_ABORTED; } CHIP_ERROR TCPEndPointImplLwIP::GetInterfaceId(InterfaceId * retInterface) @@ -234,21 +209,12 @@ CHIP_ERROR TCPEndPointImplLwIP::SendQueuedImpl(bool queueWasEmpty) CHIP_ERROR TCPEndPointImplLwIP::EnableNoDelay() { VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE); - - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - CHIP_ERROR res = CHIP_ERROR_CONNECTION_ABORTED; if (mTCP != nullptr) { - tcp_nagle_disable(mTCP); - res = CHIP_NO_ERROR; + RunOnTCPIP([this]() { tcp_nagle_disable(mTCP); }); + return CHIP_NO_ERROR; } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - - return res; + return CHIP_ERROR_CONNECTION_ABORTED; } CHIP_ERROR TCPEndPointImplLwIP::EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) @@ -257,33 +223,27 @@ CHIP_ERROR TCPEndPointImplLwIP::EnableKeepAlive(uint16_t interval, uint16_t time CHIP_ERROR res = CHIP_ERROR_NOT_IMPLEMENTED; #if LWIP_TCP_KEEPALIVE - - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - if (mTCP != NULL) + if (mTCP != nullptr) { - // Set the idle interval - mTCP->keep_idle = (uint32_t) interval * 1000; + RunOnTCPIP([this, interval, timeoutCount]() { + // Set the idle interval + mTCP->keep_idle = (uint32_t) interval * 1000; - // Set the probe retransmission interval. - mTCP->keep_intvl = (uint32_t) interval * 1000; + // Set the probe retransmission interval. + mTCP->keep_intvl = (uint32_t) interval * 1000; - // Set the probe timeout count - mTCP->keep_cnt = timeoutCount; + // Set the probe timeout count + mTCP->keep_cnt = timeoutCount; - // Enable keepalives for the connection. - ip_set_option(mTCP, SOF_KEEPALIVE); + // Enable keepalives for the connection. + ip_set_option(mTCP, SOF_KEEPALIVE); + }); res = CHIP_NO_ERROR; } else { res = CHIP_ERROR_CONNECTION_ABORTED; } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - #endif // LWIP_TCP_KEEPALIVE return res; @@ -295,24 +255,16 @@ CHIP_ERROR TCPEndPointImplLwIP::DisableKeepAlive() CHIP_ERROR res = CHIP_ERROR_NOT_IMPLEMENTED; #if LWIP_TCP_KEEPALIVE - - // Lock LwIP stack - LOCK_TCPIP_CORE(); - - if (mTCP != NULL) + if (mTCP != nullptr) { // Disable keepalives on the connection. - ip_reset_option(mTCP, SOF_KEEPALIVE); + RunOnTCPIP([this]() { ip_reset_option(mTCP, SOF_KEEPALIVE); }); res = CHIP_NO_ERROR; } else { res = CHIP_ERROR_CONNECTION_ABORTED; } - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - #endif // LWIP_TCP_KEEPALIVE return res; @@ -326,17 +278,14 @@ CHIP_ERROR TCPEndPointImplLwIP::SetUserTimeoutImpl(uint32_t userTimeoutMillis) CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl() { CHIP_ERROR err = CHIP_NO_ERROR; - - // Lock LwIP stack - LOCK_TCPIP_CORE(); - // If the connection hasn't been aborted ... - if (mTCP != NULL) + if (mTCP != nullptr) { err_t lwipErr; // Determine the current send window size. This is the maximum amount we can write to the connection. - uint16_t sendWindowSize = tcp_sndbuf(mTCP); + uint16_t sendWindowSize; + RunOnTCPIP([this, &sendWindowSize]() { sendWindowSize = tcp_sndbuf(mTCP); }); // If there's data to be sent and the send window is open... bool canSend = (RemainingToSend() > 0 && sendWindowSize > 0); @@ -368,7 +317,9 @@ CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl() // backing. Using TCP_WRITE_FLAG_COPY would eliminate this requirement, but overall // requires many more memory allocations which may be problematic when very // memory-constrained or when using pool-based allocations. - lwipErr = tcp_write(mTCP, sendData, sendLen, (canSend) ? TCP_WRITE_FLAG_MORE : 0); + lwipErr = RunOnTCPIPRet([this, sendData, sendLen, canSend]() { + return tcp_write(mTCP, sendData, sendLen, (canSend) ? TCP_WRITE_FLAG_MORE : 0); + }); if (lwipErr != ERR_OK) { err = chip::System::MapErrorLwIP(lwipErr); @@ -400,7 +351,7 @@ CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl() if (err == CHIP_NO_ERROR) { - lwipErr = tcp_output(mTCP); + lwipErr = RunOnTCPIPRet([this]() { return tcp_output(mTCP); }); if (lwipErr != ERR_OK) err = chip::System::MapErrorLwIP(lwipErr); @@ -412,19 +363,16 @@ CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl() // If in the SendShutdown state and the unsent queue is now empty, shutdown the PCB for sending. if (mState == State::kSendShutdown && (RemainingToSend() == 0)) { - lwipErr = tcp_shutdown(mTCP, 0, 1); + lwipErr = RunOnTCPIPRet([this]() { return tcp_shutdown(mTCP, 0, 1); }); if (lwipErr != ERR_OK) err = chip::System::MapErrorLwIP(lwipErr); } } } - else + { err = CHIP_ERROR_CONNECTION_ABORTED; - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - + } return err; } @@ -432,18 +380,15 @@ void TCPEndPointImplLwIP::HandleConnectCompleteImpl() {} void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) { - // Lock LwIP stack - LOCK_TCPIP_CORE(); - // If the LwIP PCB hasn't been closed yet... - if (mTCP != NULL) + if (mTCP != nullptr) { // If the endpoint was a connection endpoint (vs. a listening endpoint)... if (oldState != State::kListening) { // Prevent further callbacks for incoming data. This has the effect of instructing // LwIP to discard any further data received from the peer. - tcp_recv(mTCP, NULL); + RunOnTCPIP([this]() { tcp_recv(mTCP, NULL); }); // If entering the Closed state... if (mState == State::kClosed) @@ -460,24 +405,23 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) // when LwIP will no longer make callbacks to its user. Thus we must block further // callbacks to prevent them from happening after the endpoint has been freed. // - tcp_err(mTCP, NULL); + RunOnTCPIP([this]() { tcp_err(mTCP, nullptr); }); // If the endpoint is being closed without error, THEN call tcp_close() to close the underlying // TCP connection gracefully, preserving any in-transit send data. if (err == CHIP_NO_ERROR) { - tcp_close(mTCP); + RunOnTCPIP([this]() { tcp_close(mTCP); }); } - // OTHERWISE, call tcp_abort() to abort the TCP connection, discarding any in-transit data. else { - tcp_abort(mTCP); + RunOnTCPIP([this]() { tcp_abort(mTCP); }); } // Discard the reference to the PCB to ensure there is no further interaction with it // after this point. - mTCP = NULL; + mTCP = nullptr; mLwIPEndPointType = LwIPEndPointType::Unknown; } } @@ -485,18 +429,15 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) // OTHERWISE the endpoint was being used for listening, so simply close it. else { - tcp_close(mTCP); + RunOnTCPIP([this]() { tcp_close(mTCP); }); // Discard the reference to the PCB to ensure there is no further interaction with it // after this point. - mTCP = NULL; + mTCP = nullptr; mLwIPEndPointType = LwIPEndPointType::Unknown; } } - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - if (mState == State::kClosed) { mUnackedLength = 0; @@ -506,22 +447,14 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) CHIP_ERROR TCPEndPointImplLwIP::AckReceive(size_t len) { VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR res = CHIP_NO_ERROR; - VerifyOrReturnError(CanCastTo(len), CHIP_ERROR_INVALID_ARGUMENT); - // Lock LwIP stack - LOCK_TCPIP_CORE(); - if (mTCP != nullptr) - tcp_recved(mTCP, static_cast(len)); - else - res = CHIP_ERROR_CONNECTION_ABORTED; - - // Unlock LwIP stack - UNLOCK_TCPIP_CORE(); - - return res; + { + RunOnTCPIP([this, len]() { tcp_recved(mTCP, static_cast(len)); }); + return CHIP_NO_ERROR; + } + return CHIP_ERROR_CONNECTION_ABORTED; } #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT @@ -542,14 +475,10 @@ uint16_t TCPEndPointImplLwIP::RemainingToSend() { return 0; } - else - { - // We can never have reported more unacked data than there is pending - // in the send queue! This would indicate a critical accounting bug. - VerifyOrDie(mUnackedLength <= mSendQueue->TotalLength()); - - return static_cast(mSendQueue->TotalLength() - mUnackedLength); - } + // We can never have reported more unacked data than there is pending + // in the send queue! This would indicate a critical accounting bug. + VerifyOrDie(mUnackedLength <= mSendQueue->TotalLength()); + return static_cast(mSendQueue->TotalLength() - mUnackedLength); } TCPEndPointImplLwIP::BufferOffset TCPEndPointImplLwIP::FindStartOfUnsent() @@ -596,17 +525,17 @@ TCPEndPointImplLwIP::BufferOffset TCPEndPointImplLwIP::FindStartOfUnsent() CHIP_ERROR TCPEndPointImplLwIP::GetPCB(IPAddressType addrType) { // IMMPORTANT: This method MUST be called with the LwIP stack LOCKED! - if (mTCP == NULL) + if (mTCP == nullptr) { switch (addrType) { case IPAddressType::kIPv6: - mTCP = tcp_new_ip_type(IPADDR_TYPE_V6); + RunOnTCPIP([this]() { mTCP = tcp_new_ip_type(IPADDR_TYPE_V6); }); break; #if INET_CONFIG_ENABLE_IPV4 case IPAddressType::kIPv4: - mTCP = tcp_new_ip_type(IPADDR_TYPE_V4); + RunOnTCPIP([this]() { mTCP = tcp_new_ip_type(IPADDR_TYPE_V4); }); break; #endif // INET_CONFIG_ENABLE_IPV4 @@ -614,14 +543,11 @@ CHIP_ERROR TCPEndPointImplLwIP::GetPCB(IPAddressType addrType) return INET_ERROR_WRONG_ADDRESS_TYPE; } - if (mTCP == NULL) + if (mTCP == nullptr) { return CHIP_ERROR_NO_MEMORY; } - else - { - mLwIPEndPointType = LwIPEndPointType::TCP; - } + mLwIPEndPointType = LwIPEndPointType::TCP; } else { @@ -659,7 +585,7 @@ void TCPEndPointImplLwIP::HandleDataSent(uint16_t lenSent) DoClose(CHIP_ERROR_UNEXPECTED_EVENT, false); return; } - else if (mSendQueue.IsNull()) + if (mSendQueue.IsNull()) { ChipLogError(Inet, "Got ACK for %d bytes but data backing gone", (int) lenSent); DoClose(CHIP_ERROR_UNEXPECTED_EVENT, false); @@ -694,16 +620,20 @@ void TCPEndPointImplLwIP::HandleDataSent(uint16_t lenSent) MarkActive(); // If requested, call the app's OnDataSent callback. - if (OnDataSent != NULL) + if (OnDataSent != nullptr) + { OnDataSent(this, lenSent); - + } // If unsent data exists, attempt to send it now... if (RemainingToSend() > 0) + { DriveSending(); - + } // If in the closing state and the send queue is now empty, attempt to transition to closed. if ((mState == State::kClosing) && (RemainingToSend() == 0)) + { DoClose(CHIP_NO_ERROR, false); + } } } @@ -739,14 +669,19 @@ void TCPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && buf) // 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 != NULL) + if (mState == State::kConnected && OnPeerClose != nullptr) + { mState = State::kReceiveShutdown; + } else + { mState = State::kClosing; - + } // Call the app's OnPeerClose. if (OnPeerClose != NULL) + { OnPeerClose(this); + } } // Drive the received data into the app. @@ -763,45 +698,57 @@ void TCPEndPointImplLwIP::HandleIncomingConnection(TCPEndPoint * conEP) if (mState == State::kListening) { // If there's no callback available, fail with an error. - if (OnConnectionReceived == NULL) + if (OnConnectionReceived == nullptr) + { err = CHIP_ERROR_NO_CONNECTION_HANDLER; - + } // Extract the peer's address information. if (err == CHIP_NO_ERROR) + { err = conEP->GetPeerInfo(&peerAddr, &peerPort); - + } // If successful, call the app's callback function. if (err == CHIP_NO_ERROR) + { OnConnectionReceived(this, conEP, peerAddr, peerPort); - + } // Otherwise clean up and call the app's error callback. - else if (OnAcceptError != NULL) + else if (OnAcceptError != nullptr) + { OnAcceptError(this, err); + } } else + { err = CHIP_ERROR_INCORRECT_STATE; - + } // If something failed above, abort and free the connection end point. if (err != CHIP_NO_ERROR) + { conEP->Free(); + } } void TCPEndPointImplLwIP::HandleError(CHIP_ERROR err) { if (mState == State::kListening) { - if (OnAcceptError != NULL) + if (OnAcceptError != nullptr) + { OnAcceptError(this, err); + } } else + { DoClose(err, false); + } } err_t TCPEndPointImplLwIP::LwIPHandleConnectComplete(void * arg, struct tcp_pcb * tpcb, err_t lwipErr) { err_t res = ERR_OK; - if (arg != NULL) + if (arg != nullptr) { TCPEndPointImplLwIP * ep = static_cast(arg); @@ -825,11 +772,13 @@ err_t TCPEndPointImplLwIP::LwIPHandleConnectComplete(void * arg, struct tcp_pcb } } else + { res = ERR_ABRT; - + } if (res != ERR_OK) + { tcp_abort(tpcb); - + } return res; } @@ -837,10 +786,10 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p { CHIP_ERROR err = chip::System::MapErrorLwIP(lwipErr); - if (arg != NULL) + if (arg != nullptr) { TCPEndPointImplLwIP * listenEP = static_cast(arg); - TCPEndPointImplLwIP * conEP = NULL; + TCPEndPointImplLwIP * conEP = nullptr; System::Layer & lSystemLayer = listenEP->GetSystemLayer(); // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter. @@ -856,8 +805,9 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (err == CHIP_NO_ERROR) { TCPEndPoint * connectEndPoint = nullptr; - err = listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); - conEP = static_cast(connectEndPoint); + err = lSystemLayer.RunOnMatterContext( + [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); }); + conEP = static_cast(connectEndPoint); } // Ensure that TCP timers have been started @@ -918,24 +868,23 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p } } else + { err = CHIP_ERROR_CONNECTION_ABORTED; + } - if (err != CHIP_NO_ERROR && tpcb != NULL) + if (err != CHIP_NO_ERROR && tpcb != nullptr) { tcp_abort(tpcb); return ERR_ABRT; } - else - { - return ERR_OK; - } + return ERR_OK; } err_t TCPEndPointImplLwIP::LwIPHandleDataReceived(void * arg, struct tcp_pcb * tpcb, struct pbuf * p, err_t _err) { err_t res = ERR_OK; - if (arg != NULL) + if (arg != nullptr) { TCPEndPointImplLwIP * ep = static_cast(arg); @@ -952,7 +901,9 @@ err_t TCPEndPointImplLwIP::LwIPHandleDataReceived(void * arg, struct tcp_pcb * t } } else + { res = ERR_ABRT; + } if (res != ERR_OK) { @@ -970,7 +921,7 @@ err_t TCPEndPointImplLwIP::LwIPHandleDataSent(void * arg, struct tcp_pcb * tpcb, { err_t res = ERR_OK; - if (arg != NULL) + if (arg != nullptr) { TCPEndPointImplLwIP * ep = static_cast(arg); @@ -987,17 +938,21 @@ err_t TCPEndPointImplLwIP::LwIPHandleDataSent(void * arg, struct tcp_pcb * tpcb, } } else + { res = ERR_ABRT; + } if (res != ERR_OK) + { tcp_abort(tpcb); + } return res; } void TCPEndPointImplLwIP::LwIPHandleError(void * arg, err_t lwipErr) { - if (arg != NULL) + if (arg != nullptr) { TCPEndPointImplLwIP * ep = static_cast(arg); System::LayerFreeRTOS & lSystemLayer = static_cast(ep->GetSystemLayer()); @@ -1007,7 +962,7 @@ void TCPEndPointImplLwIP::LwIPHandleError(void * arg, err_t lwipErr) // as a means to signal the other thread that the connection has been aborted. The implication // of this is that the mTCP field is shared state between the two threads and thus must only be // accessed with the LwIP lock held. - ep->mTCP = NULL; + ep->mTCP = nullptr; ep->mLwIPEndPointType = LwIPEndPointType::Unknown; // Post callback to HandleError. @@ -1017,7 +972,9 @@ void TCPEndPointImplLwIP::LwIPHandleError(void * arg, err_t lwipErr) ep->Release(); }); if (err != CHIP_NO_ERROR) + { ep->Release(); + } } } diff --git a/src/platform/PlatformEventSupport.cpp b/src/platform/PlatformEventSupport.cpp index 947a7f2fe691f8..e96f2e4b2254b3 100644 --- a/src/platform/PlatformEventSupport.cpp +++ b/src/platform/PlatformEventSupport.cpp @@ -44,5 +44,15 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T return PlatformMgr().StartChipTimer(delay); } +void PlatformEventing::LockMatterStack(System::Layer & aLayer) +{ + PlatformMgr().LockChipStack(); +} + +void PlatformEventing::UnlockMatterStack(System::Layer & aLayer) +{ + PlatformMgr().UnlockChipStack(); +} + } // namespace System } // namespace chip diff --git a/src/system/PlatformEventSupport.h b/src/system/PlatformEventSupport.h index 9c0e85a545cd44..5fb47ebf852076 100644 --- a/src/system/PlatformEventSupport.h +++ b/src/system/PlatformEventSupport.h @@ -31,6 +31,8 @@ class PlatformEventing public: static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge); static CHIP_ERROR StartTimer(System::Layer & aLayer, System::Clock::Timeout aTimeout); + static void LockMatterStack(System::Layer & aLayer); + static void UnlockMatterStack(System::Layer & aLayer); }; } // namespace System diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index fe1090b765f1b5..cdaf6096d3eda7 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -31,5 +31,14 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) return lReturn; } +CHIP_ERROR Layer::RunOnMatterContext(std::function func) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + PlatformEventing::LockMatterStack(*this); + err = func(); + PlatformEventing::UnlockMatterStack(*this); + return err; +} + } // namespace System } // namespace chip diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 9314ccd5609786..b5d9068433b2ca 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -25,6 +25,7 @@ #pragma once +#include #include #include @@ -219,6 +220,8 @@ class DLL_EXPORT Layer return ScheduleLambdaBridge(std::move(bridge)); } + CHIP_ERROR RunOnMatterContext(std::function); + private: CHIP_ERROR ScheduleLambdaBridge(LambdaBridge && bridge); From 5066826df325e4aef5040dd0bb928073e0442b38 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 6 Jan 2025 15:54:43 +0800 Subject: [PATCH 02/29] fix memory leak when the pbuf_alloc gets a packet buffer with chained buffers --- src/system/SystemPacketBuffer.cpp | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index f644d1a23eb323..bd42ace46b5cfb 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -634,12 +634,15 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese SYSTEM_STATS_INCREMENT(chip::System::Stats::kSystemLayer_NumPacketBufs); lPacket->payload = lPacket->ReserveStart() + aReservedSize; - lPacket->len = lPacket->tot_len = 0; - lPacket->next = nullptr; - lPacket->ref = 1; #if CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP lPacket->alloc_size = lAllocSize; #endif + // Set current packet and chained buffers' length and total length to 0 + for (PacketBuffer *pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) + { + pktBuf->len = pktBuf->tot_len = 0; + pktBuf->ref = 1; + } return PacketBufferHandle(lPacket); } @@ -652,14 +655,28 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize); if (buffer.mBuffer != nullptr) { - memcpy(buffer.mBuffer->payload, aData, aDataSize); #if CHIP_SYSTEM_CONFIG_USE_LWIP // Checks in the New() call catch buffer allocations greater // than UINT16_MAX for LwIP based platforms. - buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast(aDataSize); + buffer.mBuffer->tot_len = static_cast(aDataSize); #else - buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize; + buffer.mBuffer->tot_len = aDataSize; #endif + PacketBuffer *currentBuffer = buffer.mBuffer; + const uint8_t *dataPtr = static_cast(aData); + while (aDataSize > 0) + { + size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength(); + memcpy(currentBuffer->payload, aData, copyLen); + aDataSize -= copyLen; + dataPtr += copyLen; +#if CHIP_SYSTEM_CONFIG_USE_LWIP + currentBuffer->len = static_cast(copyLen); +#else + currentBuffer->len = copyLen; +#endif + currentBuffer = currentBuffer->ChainedBuffer(); + } } return buffer; } From 52566571e337494ca98b35f2ec2f29a21e44bde5 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 6 Jan 2025 08:46:59 +0000 Subject: [PATCH 03/29] Restyled by clang-format --- src/system/SystemPacketBuffer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index bd42ace46b5cfb..c3bcf8f18f7827 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -638,10 +638,10 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese lPacket->alloc_size = lAllocSize; #endif // Set current packet and chained buffers' length and total length to 0 - for (PacketBuffer *pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) + for (PacketBuffer * pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) { pktBuf->len = pktBuf->tot_len = 0; - pktBuf->ref = 1; + pktBuf->ref = 1; } return PacketBufferHandle(lPacket); @@ -662,8 +662,8 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD #else buffer.mBuffer->tot_len = aDataSize; #endif - PacketBuffer *currentBuffer = buffer.mBuffer; - const uint8_t *dataPtr = static_cast(aData); + PacketBuffer * currentBuffer = buffer.mBuffer; + const uint8_t * dataPtr = static_cast(aData); while (aDataSize > 0) { size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength(); From 267b09da9ade8be17c37edbf9d95be826bdd85d7 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 6 Jan 2025 17:18:29 +0800 Subject: [PATCH 04/29] fix build issue --- src/system/SystemPacketBuffer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index c3bcf8f18f7827..38608c0d8e091d 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -667,7 +667,7 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD while (aDataSize > 0) { size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength(); - memcpy(currentBuffer->payload, aData, copyLen); + memcpy(currentBuffer->payload, dataPtr, copyLen); aDataSize -= copyLen; dataPtr += copyLen; #if CHIP_SYSTEM_CONFIG_USE_LWIP From 7ecbf7bc6e2e5009875ac264096fc032d700bc10 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 7 Jan 2025 16:05:57 +0800 Subject: [PATCH 05/29] Try to fix CI build errors --- src/system/SystemPacketBuffer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 38608c0d8e091d..3cd1bbf57ea052 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -636,6 +636,9 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese lPacket->payload = lPacket->ReserveStart() + aReservedSize; #if CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP lPacket->alloc_size = lAllocSize; +#endif +#if !CHIP_SYSTEM_CONFIG_USE_LWIP + lPacket->next = nullptr; #endif // Set current packet and chained buffers' length and total length to 0 for (PacketBuffer * pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) @@ -664,7 +667,7 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD #endif PacketBuffer * currentBuffer = buffer.mBuffer; const uint8_t * dataPtr = static_cast(aData); - while (aDataSize > 0) + while (currentBuffer && aDataSize > 0) { size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength(); memcpy(currentBuffer->payload, dataPtr, copyLen); From ead181f35fc05e7bc2d3c32f6855674f0c26da9f Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 7 Jan 2025 16:09:43 +0800 Subject: [PATCH 06/29] move RunOnMatterContext to LayerFreeRTOS --- src/inet/TCPEndPointImplLwIP.cpp | 6 +++--- src/system/SystemLayer.cpp | 4 +++- src/system/SystemLayer.h | 3 +-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 2d991fcf295565..9eccddfe6be0ad 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -788,9 +788,9 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (arg != nullptr) { - TCPEndPointImplLwIP * listenEP = static_cast(arg); - TCPEndPointImplLwIP * conEP = nullptr; - System::Layer & lSystemLayer = listenEP->GetSystemLayer(); + TCPEndPointImplLwIP * listenEP = static_cast(arg); + TCPEndPointImplLwIP * conEP = nullptr; + System::LayerFreeRTOS & lSystemLayer = static_cast(istenEP->GetSystemLayer()); // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter. tcp_accepted(listenEP->mTCP); diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index cdaf6096d3eda7..5d2c6a8fe0f80d 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -31,7 +31,8 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) return lReturn; } -CHIP_ERROR Layer::RunOnMatterContext(std::function func) +#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT +CHIP_ERROR LayerFreeRTOS::RunOnMatterContext(std::function func) { CHIP_ERROR err = CHIP_NO_ERROR; PlatformEventing::LockMatterStack(*this); @@ -39,6 +40,7 @@ CHIP_ERROR Layer::RunOnMatterContext(std::function func) PlatformEventing::UnlockMatterStack(*this); return err; } +#endif // CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT } // namespace System } // namespace chip diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index b5d9068433b2ca..8742b3d85613d3 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -220,8 +220,6 @@ class DLL_EXPORT Layer return ScheduleLambdaBridge(std::move(bridge)); } - CHIP_ERROR RunOnMatterContext(std::function); - private: CHIP_ERROR ScheduleLambdaBridge(LambdaBridge && bridge); @@ -234,6 +232,7 @@ class DLL_EXPORT Layer class LayerFreeRTOS : public Layer { + CHIP_ERROR RunOnMatterContext(std::function); }; #endif // CHIP_SYSTEM_CONFIG_USE_LWIP From c3dc926fd20c83a89385b976a16b20e212bcbf76 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 7 Jan 2025 16:12:33 +0800 Subject: [PATCH 07/29] add macro for PlatformEventing::LockMatterStack/UnlockMatterStack --- src/platform/PlatformEventSupport.cpp | 2 ++ src/system/PlatformEventSupport.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/platform/PlatformEventSupport.cpp b/src/platform/PlatformEventSupport.cpp index e96f2e4b2254b3..cfa598db340642 100644 --- a/src/platform/PlatformEventSupport.cpp +++ b/src/platform/PlatformEventSupport.cpp @@ -44,6 +44,7 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T return PlatformMgr().StartChipTimer(delay); } +#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT void PlatformEventing::LockMatterStack(System::Layer & aLayer) { PlatformMgr().LockChipStack(); @@ -53,6 +54,7 @@ void PlatformEventing::UnlockMatterStack(System::Layer & aLayer) { PlatformMgr().UnlockChipStack(); } +#endif // CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT } // namespace System } // namespace chip diff --git a/src/system/PlatformEventSupport.h b/src/system/PlatformEventSupport.h index 5fb47ebf852076..4fb7045d1b21e7 100644 --- a/src/system/PlatformEventSupport.h +++ b/src/system/PlatformEventSupport.h @@ -31,8 +31,10 @@ class PlatformEventing public: static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge); static CHIP_ERROR StartTimer(System::Layer & aLayer, System::Clock::Timeout aTimeout); +#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT static void LockMatterStack(System::Layer & aLayer); static void UnlockMatterStack(System::Layer & aLayer); +#endif }; } // namespace System From 2e52e5e200365bc8333883b266a42033d6a0f85f Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 7 Jan 2025 16:19:44 +0800 Subject: [PATCH 08/29] make the new function public in LayerFreeRTOS --- src/system/SystemLayer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 8742b3d85613d3..b8112d9bd94edd 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -232,6 +232,7 @@ class DLL_EXPORT Layer class LayerFreeRTOS : public Layer { +public: CHIP_ERROR RunOnMatterContext(std::function); }; From cca01f2432bd61661800ad9c73f03b15165719b3 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 7 Jan 2025 16:21:35 +0800 Subject: [PATCH 09/29] fix typo --- src/inet/TCPEndPointImplLwIP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 9eccddfe6be0ad..0dc56d2dbfbdf6 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -790,7 +790,7 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p { TCPEndPointImplLwIP * listenEP = static_cast(arg); TCPEndPointImplLwIP * conEP = nullptr; - System::LayerFreeRTOS & lSystemLayer = static_cast(istenEP->GetSystemLayer()); + System::LayerFreeRTOS & lSystemLayer = static_cast(listenEP->GetSystemLayer()); // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter. tcp_accepted(listenEP->mTCP); From 0b6905ef5a9451fcb39f81662fccfa5469959c32 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Thu, 9 Jan 2025 10:56:08 +0800 Subject: [PATCH 10/29] add some comments in Packet buffer --- src/system/SystemPacketBuffer.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 3cd1bbf57ea052..25d14687a4f0d9 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -638,9 +638,12 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese lPacket->alloc_size = lAllocSize; #endif #if !CHIP_SYSTEM_CONFIG_USE_LWIP + // For non-lwip packet buffer, the allocated packet buffer will not have chained buffers, set it to null to avoid potential + // issues. lPacket->next = nullptr; #endif - // Set current packet and chained buffers' length and total length to 0 + // Set ther length and total length of the head packet buffer and all the chained packet buffers to 0 + // as we don't put any data in them. And set the ref to 1 for all the buffers. for (PacketBuffer * pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) { pktBuf->len = pktBuf->tot_len = 0; @@ -665,6 +668,8 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD #else buffer.mBuffer->tot_len = aDataSize; #endif + // Copy the data to the first packet buffer, if the data size is larger than the MaxDataLength, copy the remaining + // data to the chained packet buffers. PacketBuffer * currentBuffer = buffer.mBuffer; const uint8_t * dataPtr = static_cast(aData); while (currentBuffer && aDataSize > 0) @@ -728,7 +733,7 @@ void PacketBuffer::Free(PacketBuffer * aPacket) #elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP chip::Platform::MemoryFree(aPacket); #endif - aPacket = lNextPacket; + aPacket = lNextPacket; } else { From a331147297b0d8589563a9dc031f47bba67a20b7 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 9 Jan 2025 02:57:17 +0000 Subject: [PATCH 11/29] Restyled by clang-format --- src/system/SystemPacketBuffer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 25d14687a4f0d9..1e139bb749e1ab 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -733,7 +733,7 @@ void PacketBuffer::Free(PacketBuffer * aPacket) #elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP chip::Platform::MemoryFree(aPacket); #endif - aPacket = lNextPacket; + aPacket = lNextPacket; } else { From b14b2c7247f85f23dabe8abefb22427fe58b82f6 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Fri, 10 Jan 2025 10:57:52 +0800 Subject: [PATCH 12/29] Add macro for the RunWithMatterContextLock function --- src/inet/TCPEndPointImplLwIP.cpp | 18 ++++++++++++------ src/platform/PlatformEventSupport.cpp | 8 ++++---- src/system/PlatformEventSupport.h | 6 +++--- src/system/SystemLayer.cpp | 10 +++++----- src/system/SystemLayer.h | 6 ++++-- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 0dc56d2dbfbdf6..9021dcfa350047 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -788,9 +788,9 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (arg != nullptr) { - TCPEndPointImplLwIP * listenEP = static_cast(arg); - TCPEndPointImplLwIP * conEP = nullptr; - System::LayerFreeRTOS & lSystemLayer = static_cast(listenEP->GetSystemLayer()); + TCPEndPointImplLwIP * listenEP = static_cast(arg); + TCPEndPointImplLwIP * conEP = nullptr; + System::Layer & lSystemLayer = listenEP->GetSystemLayer(); // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter. tcp_accepted(listenEP->mTCP); @@ -805,8 +805,14 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (err == CHIP_NO_ERROR) { TCPEndPoint * connectEndPoint = nullptr; - err = lSystemLayer.RunOnMatterContext( +#if CHIP_SYSTEM_CONFIG_NO_LOCKING + // TODO This should be in Matter context but we cannot use SystemLayer.ScheduleLambda() here as the allocated endpoint + // will be used in the following. + err = listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); +#else + err = lSystemLayer.RunWithMatterContextLock( [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); }); +#endif conEP = static_cast(connectEndPoint); } @@ -954,8 +960,8 @@ void TCPEndPointImplLwIP::LwIPHandleError(void * arg, err_t lwipErr) { if (arg != nullptr) { - TCPEndPointImplLwIP * ep = static_cast(arg); - System::LayerFreeRTOS & lSystemLayer = static_cast(ep->GetSystemLayer()); + TCPEndPointImplLwIP * ep = static_cast(arg); + System::Layer & lSystemLayer = ep->GetSystemLayer(); // At this point LwIP has already freed the PCB. Since the thread that owns the TCPEndPoint may // try to use the PCB before it receives the TCPError event posted below, we set the PCB to NULL diff --git a/src/platform/PlatformEventSupport.cpp b/src/platform/PlatformEventSupport.cpp index cfa598db340642..13929f992bff06 100644 --- a/src/platform/PlatformEventSupport.cpp +++ b/src/platform/PlatformEventSupport.cpp @@ -44,17 +44,17 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T return PlatformMgr().StartChipTimer(delay); } -#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT -void PlatformEventing::LockMatterStack(System::Layer & aLayer) +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING +void PlatformEventing::LockMatterStack() { PlatformMgr().LockChipStack(); } -void PlatformEventing::UnlockMatterStack(System::Layer & aLayer) +void PlatformEventing::UnlockMatterStack() { PlatformMgr().UnlockChipStack(); } -#endif // CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT +#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING } // namespace System } // namespace chip diff --git a/src/system/PlatformEventSupport.h b/src/system/PlatformEventSupport.h index 4fb7045d1b21e7..221feaee5348e1 100644 --- a/src/system/PlatformEventSupport.h +++ b/src/system/PlatformEventSupport.h @@ -31,9 +31,9 @@ class PlatformEventing public: static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge); static CHIP_ERROR StartTimer(System::Layer & aLayer, System::Clock::Timeout aTimeout); -#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT - static void LockMatterStack(System::Layer & aLayer); - static void UnlockMatterStack(System::Layer & aLayer); +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING + static void LockMatterStack(); + static void UnlockMatterStack(); #endif }; diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index 5d2c6a8fe0f80d..628fab6430341f 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -31,16 +31,16 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) return lReturn; } -#if CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT -CHIP_ERROR LayerFreeRTOS::RunOnMatterContext(std::function func) +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING +CHIP_ERROR Layer::RunWithMatterContextLock(std::function func) { CHIP_ERROR err = CHIP_NO_ERROR; - PlatformEventing::LockMatterStack(*this); + PlatformEventing::LockMatterStack(); err = func(); - PlatformEventing::UnlockMatterStack(*this); + PlatformEventing::UnlockMatterStack(); return err; } -#endif // CHIP_SYSTEM_CONFIG_USE_LWIP || CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT +#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING } // namespace System } // namespace chip diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index b8112d9bd94edd..a381b011ea4c26 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -220,6 +220,10 @@ class DLL_EXPORT Layer return ScheduleLambdaBridge(std::move(bridge)); } +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING + CHIP_ERROR RunWithMatterContextLock(std::function); +#endif + private: CHIP_ERROR ScheduleLambdaBridge(LambdaBridge && bridge); @@ -232,8 +236,6 @@ class DLL_EXPORT Layer class LayerFreeRTOS : public Layer { -public: - CHIP_ERROR RunOnMatterContext(std::function); }; #endif // CHIP_SYSTEM_CONFIG_USE_LWIP From 4613b8f36dbe63518bd6c690cfcf290c42a62a79 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Fri, 10 Jan 2025 11:22:14 +0800 Subject: [PATCH 13/29] add some comment for avoiding dead lock with the new added function --- src/system/SystemLayer.cpp | 4 ++-- src/system/SystemLayer.h | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index 628fab6430341f..067032efb7b5db 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -32,11 +32,11 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) } #if !CHIP_SYSTEM_CONFIG_NO_LOCKING -CHIP_ERROR Layer::RunWithMatterContextLock(std::function func) +CHIP_ERROR Layer::RunWithMatterContextLock(std::function nonBlockingFunc) { CHIP_ERROR err = CHIP_NO_ERROR; PlatformEventing::LockMatterStack(); - err = func(); + err = nonBlockingFunc(); PlatformEventing::UnlockMatterStack(); return err; } diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index a381b011ea4c26..1b9c0e38668b5a 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -221,7 +221,17 @@ class DLL_EXPORT Layer } #if !CHIP_SYSTEM_CONFIG_NO_LOCKING - CHIP_ERROR RunWithMatterContextLock(std::function); + /** + * @brief + * Call the func with Matter context locked + * + * CRITICAL: The function should be non-blocking to avoid dead lock. + * + * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock + * + * @retval The return value of the non-blocking function + */ + CHIP_ERROR RunWithMatterContextLock(std::function nonBlockingFunc); #endif private: From 58b91ab7a6ccb640fa2241854bc0e0251598bc89 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 13 Jan 2025 10:01:32 +0800 Subject: [PATCH 14/29] review changes and revert packet buffer changes --- src/inet/TCPEndPointImplLwIP.cpp | 11 +++++---- src/system/SystemLayer.h | 2 +- src/system/SystemPacketBuffer.cpp | 37 +++++-------------------------- 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 9021dcfa350047..f1992b63119506 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -39,6 +39,11 @@ static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later"); +static_assert(!CHIP_SYSTEM_CONFIG_NO_LOCKING, + "CHIP_SYSTEM_CONFIG_NO_LOCKING not supported along with using LwIP for TCP, because handling incoming connection " + "attempts needs to be done synchronously in LwIP, but part of the work for it needs to happen in the Matter context. " + " If support for this configuration is needed, this part needs to be figured out."); + namespace chip { namespace Inet { @@ -805,14 +810,8 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (err == CHIP_NO_ERROR) { TCPEndPoint * connectEndPoint = nullptr; -#if CHIP_SYSTEM_CONFIG_NO_LOCKING - // TODO This should be in Matter context but we cannot use SystemLayer.ScheduleLambda() here as the allocated endpoint - // will be used in the following. - err = listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); -#else err = lSystemLayer.RunWithMatterContextLock( [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); }); -#endif conEP = static_cast(connectEndPoint); } diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 1b9c0e38668b5a..917d7814b230b3 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -227,7 +227,7 @@ class DLL_EXPORT Layer * * CRITICAL: The function should be non-blocking to avoid dead lock. * - * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock + * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock held. * * @retval The return value of the non-blocking function */ diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 1e139bb749e1ab..f644d1a23eb323 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -634,21 +634,12 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese SYSTEM_STATS_INCREMENT(chip::System::Stats::kSystemLayer_NumPacketBufs); lPacket->payload = lPacket->ReserveStart() + aReservedSize; + lPacket->len = lPacket->tot_len = 0; + lPacket->next = nullptr; + lPacket->ref = 1; #if CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP lPacket->alloc_size = lAllocSize; #endif -#if !CHIP_SYSTEM_CONFIG_USE_LWIP - // For non-lwip packet buffer, the allocated packet buffer will not have chained buffers, set it to null to avoid potential - // issues. - lPacket->next = nullptr; -#endif - // Set ther length and total length of the head packet buffer and all the chained packet buffers to 0 - // as we don't put any data in them. And set the ref to 1 for all the buffers. - for (PacketBuffer * pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer()) - { - pktBuf->len = pktBuf->tot_len = 0; - pktBuf->ref = 1; - } return PacketBufferHandle(lPacket); } @@ -661,30 +652,14 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize); if (buffer.mBuffer != nullptr) { + memcpy(buffer.mBuffer->payload, aData, aDataSize); #if CHIP_SYSTEM_CONFIG_USE_LWIP // Checks in the New() call catch buffer allocations greater // than UINT16_MAX for LwIP based platforms. - buffer.mBuffer->tot_len = static_cast(aDataSize); -#else - buffer.mBuffer->tot_len = aDataSize; -#endif - // Copy the data to the first packet buffer, if the data size is larger than the MaxDataLength, copy the remaining - // data to the chained packet buffers. - PacketBuffer * currentBuffer = buffer.mBuffer; - const uint8_t * dataPtr = static_cast(aData); - while (currentBuffer && aDataSize > 0) - { - size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength(); - memcpy(currentBuffer->payload, dataPtr, copyLen); - aDataSize -= copyLen; - dataPtr += copyLen; -#if CHIP_SYSTEM_CONFIG_USE_LWIP - currentBuffer->len = static_cast(copyLen); + buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast(aDataSize); #else - currentBuffer->len = copyLen; + buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize; #endif - currentBuffer = currentBuffer->ChainedBuffer(); - } } return buffer; } From 7f513d69ff4addbedb3d3dfd69aca626817535f7 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 13 Jan 2025 10:53:20 +0800 Subject: [PATCH 15/29] Add PlatformLockSupport for system layer --- src/platform/BUILD.gn | 5 +++ src/platform/PlatformEventSupport.cpp | 12 ------- src/platform/PlatformLockSupport.cpp | 48 +++++++++++++++++++++++++++ src/system/BUILD.gn | 4 +++ src/system/PlatformEventSupport.h | 4 --- src/system/PlatformLockSupport.h | 36 ++++++++++++++++++++ src/system/SystemLayer.cpp | 7 ++-- 7 files changed, 98 insertions(+), 18 deletions(-) create mode 100644 src/platform/PlatformLockSupport.cpp create mode 100644 src/system/PlatformLockSupport.h diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn index bc7f10f430e661..943b78e4a68323 100644 --- a/src/platform/BUILD.gn +++ b/src/platform/BUILD.gn @@ -18,6 +18,7 @@ import("//build_overrides/pigweed.gni") import("${build_root}/config/linux/pkg_config.gni") import("${chip_root}/build/chip/buildconfig_header.gni") +import("${chip_root}/src/system/system.gni") import("device.gni") @@ -526,6 +527,10 @@ if (chip_device_platform != "none") { "RuntimeOptionsProvider.cpp", ] + if (chip_system_config_locking != "none") { + sources += [ "PlatformLockSupport.cpp" ] + } + # Linux has its own NetworkCommissioningThreadDriver if (chip_enable_openthread && chip_device_platform != "linux" && chip_device_platform != "tizen" && chip_device_platform != "webos") { diff --git a/src/platform/PlatformEventSupport.cpp b/src/platform/PlatformEventSupport.cpp index 13929f992bff06..947a7f2fe691f8 100644 --- a/src/platform/PlatformEventSupport.cpp +++ b/src/platform/PlatformEventSupport.cpp @@ -44,17 +44,5 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T return PlatformMgr().StartChipTimer(delay); } -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING -void PlatformEventing::LockMatterStack() -{ - PlatformMgr().LockChipStack(); -} - -void PlatformEventing::UnlockMatterStack() -{ - PlatformMgr().UnlockChipStack(); -} -#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING - } // namespace System } // namespace chip diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp new file mode 100644 index 00000000000000..61a0d18af0c6a2 --- /dev/null +++ b/src/platform/PlatformLockSupport.cpp @@ -0,0 +1,48 @@ +/* + * + * Copyright (c) 2020 Project CHIP Authors + * Copyright (c) 2018 Nest Labs, Inc. + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * Provides implementations of the CHIP System Layer platform + * event functions that are suitable for use on all platforms. + */ +/* this file behaves like a config.h, comes first */ +#include +#include +#include + +namespace chip { +namespace System { + +using namespace ::chip::DeviceLayer; + +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING +void PlatformLocking::LockMatterStack() +{ + PlatformMgr().LockChipStack(); +} + +void PlatformLocking::UnlockMatterStack() +{ + PlatformMgr().UnlockChipStack(); +} +#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING + +} // namespace System +} // namespace chip diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn index 3fff2a61e648da..9b8096b52795bc 100644 --- a/src/system/BUILD.gn +++ b/src/system/BUILD.gn @@ -256,6 +256,10 @@ static_library("system") { ] } + if (chip_system_config_locking != "none") { + sources += [ "PlatformLockSupport.h" ] + } + cflags = [ "-Wconversion" ] public_deps = [ diff --git a/src/system/PlatformEventSupport.h b/src/system/PlatformEventSupport.h index 221feaee5348e1..9c0e85a545cd44 100644 --- a/src/system/PlatformEventSupport.h +++ b/src/system/PlatformEventSupport.h @@ -31,10 +31,6 @@ class PlatformEventing public: static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge); static CHIP_ERROR StartTimer(System::Layer & aLayer, System::Clock::Timeout aTimeout); -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING - static void LockMatterStack(); - static void UnlockMatterStack(); -#endif }; } // namespace System diff --git a/src/system/PlatformLockSupport.h b/src/system/PlatformLockSupport.h new file mode 100644 index 00000000000000..ac4ad03e4b9d3c --- /dev/null +++ b/src/system/PlatformLockSupport.h @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2025 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +namespace chip { +namespace System { + +class Layer; +class Object; + +class PlatformLocking +{ +public: + static void LockMatterStack(); + static void UnlockMatterStack(); +}; + +} // namespace System +} // namespace chip diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index 067032efb7b5db..4cb411006f6e75 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -17,6 +17,9 @@ #include #include #include +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING +#include +#endif namespace chip { namespace System { @@ -35,9 +38,9 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) CHIP_ERROR Layer::RunWithMatterContextLock(std::function nonBlockingFunc) { CHIP_ERROR err = CHIP_NO_ERROR; - PlatformEventing::LockMatterStack(); + PlatformLocking::LockMatterStack(); err = nonBlockingFunc(); - PlatformEventing::UnlockMatterStack(); + PlatformLocking::UnlockMatterStack(); return err; } #endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING From 637fa245f8a8780556dd6c6d8c1dfcf34ad7b8fb Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 13 Jan 2025 02:55:09 +0000 Subject: [PATCH 16/29] Restyled by clang-format --- src/inet/TCPEndPointImplLwIP.cpp | 2 +- src/platform/PlatformLockSupport.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index f1992b63119506..56ed86488c7550 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -810,7 +810,7 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p if (err == CHIP_NO_ERROR) { TCPEndPoint * connectEndPoint = nullptr; - err = lSystemLayer.RunWithMatterContextLock( + err = lSystemLayer.RunWithMatterContextLock( [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); }); conEP = static_cast(connectEndPoint); } diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index 61a0d18af0c6a2..c259aff8d215d8 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -23,9 +23,9 @@ * event functions that are suitable for use on all platforms. */ /* this file behaves like a config.h, comes first */ +#include #include #include -#include namespace chip { namespace System { From d2e244e317dd82b392290cb03b0b447cac3a048f Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Tue, 14 Jan 2025 10:01:06 +0800 Subject: [PATCH 17/29] fix CI build --- src/platform/PlatformLockSupport.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index c259aff8d215d8..db8d15296f9434 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -23,8 +23,9 @@ * event functions that are suitable for use on all platforms. */ /* this file behaves like a config.h, comes first */ -#include #include + +#include #include namespace chip { From 05a4871111dff391f99f75e2338b5576bb1a7261 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Wed, 15 Jan 2025 14:52:48 +0800 Subject: [PATCH 18/29] review changes --- src/platform/PlatformLockSupport.cpp | 17 ++++++-------- src/system/PlatformLockSupport.h | 20 ++++++++-------- src/system/SystemLayerLock.cpp | 34 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 src/system/SystemLayerLock.cpp diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index db8d15296f9434..17b7c57a26b9c8 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -20,7 +20,7 @@ /** * @file * Provides implementations of the CHIP System Layer platform - * event functions that are suitable for use on all platforms. + * lock functions that are suitable for use on the platforms using lock. */ /* this file behaves like a config.h, comes first */ #include @@ -30,20 +30,17 @@ namespace chip { namespace System { +namespace PlatformLocking { -using namespace ::chip::DeviceLayer; - -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING -void PlatformLocking::LockMatterStack() +void LockMatterStack() { - PlatformMgr().LockChipStack(); + DeviceLayer::PlatformMgr().LockChipStack(); } -void PlatformLocking::UnlockMatterStack() +void UnlockMatterStack() { - PlatformMgr().UnlockChipStack(); + DeviceLayer::PlatformMgr().UnlockChipStack(); } -#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING - +} // namespace PlatformLocking } // namespace System } // namespace chip diff --git a/src/system/PlatformLockSupport.h b/src/system/PlatformLockSupport.h index ac4ad03e4b9d3c..1bebaa26914cfa 100644 --- a/src/system/PlatformLockSupport.h +++ b/src/system/PlatformLockSupport.h @@ -16,21 +16,19 @@ #pragma once -#include -#include +/** + * The declared functions in this header are defined in src/platform/PlatformLockSupport.cpp because of dependency cycle issues. + * The //src/platform:platform library depends on //src/lib/core:core library but the core library depends on the system library + * in this directory. + */ namespace chip { namespace System { +namespace PlatformLocking { -class Layer; -class Object; - -class PlatformLocking -{ -public: - static void LockMatterStack(); - static void UnlockMatterStack(); -}; +void LockMatterStack(); +void UnlockMatterStack(); +} // namespace PlatformLocking } // namespace System } // namespace chip diff --git a/src/system/SystemLayerLock.cpp b/src/system/SystemLayerLock.cpp new file mode 100644 index 00000000000000..b8a31b821e6d4d --- /dev/null +++ b/src/system/SystemLayerLock.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +namespace chip { +namespace System { + +CHIP_ERROR Layer::RunWithMatterContextLock(std::function nonBlockingFunc) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + PlatformLocking::LockMatterStack(); + err = nonBlockingFunc(); + PlatformLocking::UnlockMatterStack(); + return err; +} + +} // namespace System +} // namespace chip From cbf1123d070f90088abfe7d59ade05af67506c41 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Wed, 15 Jan 2025 15:03:15 +0800 Subject: [PATCH 19/29] remove some docs --- src/platform/PlatformLockSupport.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index 17b7c57a26b9c8..36a3c3b13a9a7e 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -17,11 +17,6 @@ * limitations under the License. */ -/** - * @file - * Provides implementations of the CHIP System Layer platform - * lock functions that are suitable for use on the platforms using lock. - */ /* this file behaves like a config.h, comes first */ #include From 220cf3bfd44e9d6695c980bf59caa55b95cdaf2f Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Wed, 15 Jan 2025 15:20:50 +0800 Subject: [PATCH 20/29] use closure --- src/system/SystemLayer.cpp | 14 -------------- src/system/SystemLayer.h | 17 +++++++++++++++-- src/system/SystemLayerLock.cpp | 34 ---------------------------------- 3 files changed, 15 insertions(+), 50 deletions(-) delete mode 100644 src/system/SystemLayerLock.cpp diff --git a/src/system/SystemLayer.cpp b/src/system/SystemLayer.cpp index 4cb411006f6e75..fe1090b765f1b5 100644 --- a/src/system/SystemLayer.cpp +++ b/src/system/SystemLayer.cpp @@ -17,9 +17,6 @@ #include #include #include -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING -#include -#endif namespace chip { namespace System { @@ -34,16 +31,5 @@ CHIP_ERROR Layer::ScheduleLambdaBridge(LambdaBridge && bridge) return lReturn; } -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING -CHIP_ERROR Layer::RunWithMatterContextLock(std::function nonBlockingFunc) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - PlatformLocking::LockMatterStack(); - err = nonBlockingFunc(); - PlatformLocking::UnlockMatterStack(); - return err; -} -#endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING - } // namespace System } // namespace chip diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 917d7814b230b3..f77e7944925e98 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -52,6 +52,10 @@ #include #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV +#if !CHIP_SYSTEM_CONFIG_NO_LOCKING +#include +#endif + namespace chip { namespace System { @@ -227,11 +231,20 @@ class DLL_EXPORT Layer * * CRITICAL: The function should be non-blocking to avoid dead lock. * - * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock held. + * @param[in] nonBlockingFunc The non-blocking lambda to be called with Matter stack lock held. * * @retval The return value of the non-blocking function */ - CHIP_ERROR RunWithMatterContextLock(std::function nonBlockingFunc); + template + CHIP_ERROR RunWithMatterContextLock(const Lambda & nonBlockingFunc) + { + static_assert(std::is_invocable_v, "lambda argument must be an invocable with no arguments"); + CHIP_ERROR err = CHIP_NO_ERROR; + PlatformLocking::LockMatterStack(); + err = nonBlockingFunc(); + PlatformLocking::UnlockMatterStack(); + return err; + } #endif private: diff --git a/src/system/SystemLayerLock.cpp b/src/system/SystemLayerLock.cpp deleted file mode 100644 index b8a31b821e6d4d..00000000000000 --- a/src/system/SystemLayerLock.cpp +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2021 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include - -namespace chip { -namespace System { - -CHIP_ERROR Layer::RunWithMatterContextLock(std::function nonBlockingFunc) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - PlatformLocking::LockMatterStack(); - err = nonBlockingFunc(); - PlatformLocking::UnlockMatterStack(); - return err; -} - -} // namespace System -} // namespace chip From 5af9dc58899854a8728b7087072c3ff5331e89d5 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Mon, 20 Jan 2025 10:05:27 +0800 Subject: [PATCH 21/29] Update src/platform/PlatformLockSupport.cpp Co-authored-by: Boris Zbarsky --- src/platform/PlatformLockSupport.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index 36a3c3b13a9a7e..cd8db1ea96387d 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -1,7 +1,5 @@ /* - * - * Copyright (c) 2020 Project CHIP Authors - * Copyright (c) 2018 Nest Labs, Inc. + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); From 99f823c36d680550d789f30e7db8e58f67bec33f Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 20 Jan 2025 10:26:59 +0800 Subject: [PATCH 22/29] update src/platform/PlatformLockSupport.cpp --- src/platform/PlatformLockSupport.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index cd8db1ea96387d..49fb8478840b2b 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -15,9 +15,6 @@ * limitations under the License. */ -/* this file behaves like a config.h, comes first */ -#include - #include #include From f86a812df1d9b549685b511831f453a65c014b23 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 20 Jan 2025 19:24:37 +0800 Subject: [PATCH 23/29] Revert "update src/platform/PlatformLockSupport.cpp" This reverts commit 99f823c36d680550d789f30e7db8e58f67bec33f. --- src/platform/PlatformLockSupport.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp index 49fb8478840b2b..cd8db1ea96387d 100644 --- a/src/platform/PlatformLockSupport.cpp +++ b/src/platform/PlatformLockSupport.cpp @@ -15,6 +15,9 @@ * limitations under the License. */ +/* this file behaves like a config.h, comes first */ +#include + #include #include From 0847591ab859874b6da56ed9a3b204a74de7c2fa Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Thu, 6 Feb 2025 17:25:28 +0800 Subject: [PATCH 24/29] Use a pre allocated endpoint for the TCP connection --- src/inet/TCPEndPointImplLwIP.cpp | 59 +++++++++++++++++++--------- src/inet/TCPEndPointImplLwIP.h | 5 ++- src/platform/BUILD.gn | 5 --- src/platform/PlatformLockSupport.cpp | 39 ------------------ src/system/BUILD.gn | 4 -- src/system/PlatformLockSupport.h | 34 ---------------- src/system/SystemLayer.h | 27 ------------- 7 files changed, 45 insertions(+), 128 deletions(-) delete mode 100644 src/platform/PlatformLockSupport.cpp delete mode 100644 src/system/PlatformLockSupport.h diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 56ed86488c7550..dea2aa2e2d6ba8 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -39,11 +39,6 @@ static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later"); -static_assert(!CHIP_SYSTEM_CONFIG_NO_LOCKING, - "CHIP_SYSTEM_CONFIG_NO_LOCKING not supported along with using LwIP for TCP, because handling incoming connection " - "attempts needs to be done synchronously in LwIP, but part of the work for it needs to happen in the Matter context. " - " If support for this configuration is needed, this part needs to be figured out."); - namespace chip { namespace Inet { @@ -92,13 +87,21 @@ CHIP_ERROR TCPEndPointImplLwIP::BindImpl(IPAddressType addrType, const IPAddress CHIP_ERROR TCPEndPointImplLwIP::ListenImpl(uint16_t backlog) { mLwIPEndPointType = LwIPEndPointType::TCP; - RunOnTCPIP([this]() { - // Start listening for incoming connections. - mTCP = tcp_listen(mTCP); - tcp_arg(mTCP, this); - tcp_accept(mTCP, LwIPHandleIncomingConnection); - }); - return CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; + if (!mPreAllocatedConnectEP) + { + err = GetEndPointManager().NewEndPoint(&mPreAllocatedConnectEP); + } + if (err == CHIP_NO_ERROR) + { + RunOnTCPIP([this]() { + // Start listening for incoming connections. + mTCP = tcp_listen(mTCP); + tcp_arg(mTCP, this); + tcp_accept(mTCP, LwIPHandleIncomingConnection); + }); + } + return err; } CHIP_ERROR TCPEndPointImplLwIP::ConnectImpl(const IPAddress & addr, uint16_t port, InterfaceId intfId) @@ -800,7 +803,8 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter. tcp_accepted(listenEP->mTCP); - // If we did in fact receive a connection, rather than an error, attempt to allocate an end point object. + // If we did in fact receive a connection, rather than an error, use the pre-allocated end point object for the incoming + // connection. // // NOTE: Although most of the LwIP callbacks defer the real work to happen on the endpoint's thread // (by posting events to the thread's event queue) we can't do that here because as soon as this @@ -809,10 +813,12 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p // if (err == CHIP_NO_ERROR) { - TCPEndPoint * connectEndPoint = nullptr; - err = lSystemLayer.RunWithMatterContextLock( - [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); }); - conEP = static_cast(connectEndPoint); + conEP = static_cast(listenEP->mPreAllocatedConnectEP); + if (conEP == nullptr) + { + // The listen endpoint receives a new incomming connection before it pre-allocates a new connection endpoint. + err = CHIP_ERROR_BUSY; + } } // Ensure that TCP timers have been started @@ -854,7 +860,24 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p listenEP->Release(); err = CHIP_ERROR_CONNECTION_ABORTED; conEP->Release(); // for the Retain() above - conEP->Release(); // for the implied Retain() on construction + } + else + { + // Pre-allocate another endpoint for next connection + listenEP->mPreAllocatedConnectEP = nullptr; + listenEP->Retain(); + err = lSystemLayer.ScheduleLambda([listenEP]() { + CHIP_ERROR error = listenEP->GetEndPointManager().NewEndPoint(&listenEP->mPreAllocatedConnectEP); + if (error != CHIP_NO_ERROR) + { + listenEP->HandleError(error); + } + listenEP->Release(); + }); + if (err != CHIP_NO_ERROR) + { + listenEP->Release(); + } } } diff --git a/src/inet/TCPEndPointImplLwIP.h b/src/inet/TCPEndPointImplLwIP.h index 86a151f591d303..e95f9289311d8d 100644 --- a/src/inet/TCPEndPointImplLwIP.h +++ b/src/inet/TCPEndPointImplLwIP.h @@ -41,7 +41,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP { public: TCPEndPointImplLwIP(EndPointManager & endPointManager) : - TCPEndPoint(endPointManager), mUnackedLength(0), mTCP(nullptr) + TCPEndPoint(endPointManager), mUnackedLength(0), mTCP(nullptr), mPreAllocatedConnectEP(nullptr) {} // TCPEndPoint overrides. @@ -82,6 +82,9 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP uint16_t mUnackedLength; // Amount sent but awaiting ACK. Used as a form of reference count // to hang-on to backing packet buffers until they are no longer needed. tcp_pcb * mTCP; // LwIP Transmission control protocol (TCP) control block. + // For TCP Listen endpoint, we will pre-allocate a connection endpoint to assign the incoming connection to it. + // when there is a new TCP connection established. + TCPEndPoint *mPreAllocatedConnectEP; uint16_t RemainingToSend(); BufferOffset FindStartOfUnsent(); diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn index 943b78e4a68323..bc7f10f430e661 100644 --- a/src/platform/BUILD.gn +++ b/src/platform/BUILD.gn @@ -18,7 +18,6 @@ import("//build_overrides/pigweed.gni") import("${build_root}/config/linux/pkg_config.gni") import("${chip_root}/build/chip/buildconfig_header.gni") -import("${chip_root}/src/system/system.gni") import("device.gni") @@ -527,10 +526,6 @@ if (chip_device_platform != "none") { "RuntimeOptionsProvider.cpp", ] - if (chip_system_config_locking != "none") { - sources += [ "PlatformLockSupport.cpp" ] - } - # Linux has its own NetworkCommissioningThreadDriver if (chip_enable_openthread && chip_device_platform != "linux" && chip_device_platform != "tizen" && chip_device_platform != "webos") { diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp deleted file mode 100644 index cd8db1ea96387d..00000000000000 --- a/src/platform/PlatformLockSupport.cpp +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright (c) 2025 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/* this file behaves like a config.h, comes first */ -#include - -#include -#include - -namespace chip { -namespace System { -namespace PlatformLocking { - -void LockMatterStack() -{ - DeviceLayer::PlatformMgr().LockChipStack(); -} - -void UnlockMatterStack() -{ - DeviceLayer::PlatformMgr().UnlockChipStack(); -} -} // namespace PlatformLocking -} // namespace System -} // namespace chip diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn index 9b8096b52795bc..3fff2a61e648da 100644 --- a/src/system/BUILD.gn +++ b/src/system/BUILD.gn @@ -256,10 +256,6 @@ static_library("system") { ] } - if (chip_system_config_locking != "none") { - sources += [ "PlatformLockSupport.h" ] - } - cflags = [ "-Wconversion" ] public_deps = [ diff --git a/src/system/PlatformLockSupport.h b/src/system/PlatformLockSupport.h deleted file mode 100644 index 1bebaa26914cfa..00000000000000 --- a/src/system/PlatformLockSupport.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2025 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -/** - * The declared functions in this header are defined in src/platform/PlatformLockSupport.cpp because of dependency cycle issues. - * The //src/platform:platform library depends on //src/lib/core:core library but the core library depends on the system library - * in this directory. - */ - -namespace chip { -namespace System { -namespace PlatformLocking { - -void LockMatterStack(); -void UnlockMatterStack(); - -} // namespace PlatformLocking -} // namespace System -} // namespace chip diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index f77e7944925e98..4ba72cc5e09552 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -52,10 +52,6 @@ #include #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING -#include -#endif - namespace chip { namespace System { @@ -224,29 +220,6 @@ class DLL_EXPORT Layer return ScheduleLambdaBridge(std::move(bridge)); } -#if !CHIP_SYSTEM_CONFIG_NO_LOCKING - /** - * @brief - * Call the func with Matter context locked - * - * CRITICAL: The function should be non-blocking to avoid dead lock. - * - * @param[in] nonBlockingFunc The non-blocking lambda to be called with Matter stack lock held. - * - * @retval The return value of the non-blocking function - */ - template - CHIP_ERROR RunWithMatterContextLock(const Lambda & nonBlockingFunc) - { - static_assert(std::is_invocable_v, "lambda argument must be an invocable with no arguments"); - CHIP_ERROR err = CHIP_NO_ERROR; - PlatformLocking::LockMatterStack(); - err = nonBlockingFunc(); - PlatformLocking::UnlockMatterStack(); - return err; - } -#endif - private: CHIP_ERROR ScheduleLambdaBridge(LambdaBridge && bridge); From c6dde912dd7d73c7233cdb573274ace3177d4308 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 6 Feb 2025 09:36:44 +0000 Subject: [PATCH 25/29] Restyled by clang-format --- src/inet/TCPEndPointImplLwIP.cpp | 2 +- src/inet/TCPEndPointImplLwIP.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index dea2aa2e2d6ba8..a0dc07c5027f5d 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -90,7 +90,7 @@ CHIP_ERROR TCPEndPointImplLwIP::ListenImpl(uint16_t backlog) CHIP_ERROR err = CHIP_NO_ERROR; if (!mPreAllocatedConnectEP) { - err = GetEndPointManager().NewEndPoint(&mPreAllocatedConnectEP); + err = GetEndPointManager().NewEndPoint(&mPreAllocatedConnectEP); } if (err == CHIP_NO_ERROR) { diff --git a/src/inet/TCPEndPointImplLwIP.h b/src/inet/TCPEndPointImplLwIP.h index e95f9289311d8d..7054add653fe0c 100644 --- a/src/inet/TCPEndPointImplLwIP.h +++ b/src/inet/TCPEndPointImplLwIP.h @@ -84,7 +84,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP tcp_pcb * mTCP; // LwIP Transmission control protocol (TCP) control block. // For TCP Listen endpoint, we will pre-allocate a connection endpoint to assign the incoming connection to it. // when there is a new TCP connection established. - TCPEndPoint *mPreAllocatedConnectEP; + TCPEndPoint * mPreAllocatedConnectEP; uint16_t RemainingToSend(); BufferOffset FindStartOfUnsent(); From d11b227c2bc68262636ca11290a849d05174a700 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Thu, 6 Feb 2025 17:41:43 +0800 Subject: [PATCH 26/29] revert the change in system --- src/system/SystemLayer.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 4ba72cc5e09552..9314ccd5609786 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -25,7 +25,6 @@ #pragma once -#include #include #include From 08221933a3778e5717e2ddca13e2d95cb1ed91ed Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 17 Feb 2025 17:11:30 +0800 Subject: [PATCH 27/29] Add pre allocated connection releasing when closing listen endpoint --- src/inet/TCPEndPointImplLwIP.cpp | 40 +++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index a0dc07c5027f5d..228e6d139d09c9 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -90,6 +90,10 @@ CHIP_ERROR TCPEndPointImplLwIP::ListenImpl(uint16_t backlog) CHIP_ERROR err = CHIP_NO_ERROR; if (!mPreAllocatedConnectEP) { + // Pre allocate a TCP EndPoint for TCP connection, it will be released under either of the two conditions: + // - The Listen EndPoint receives a connection and the connection will use this endpoint. The endpoint will be release when + // the connection is released. + // - The Listen Endpoint is closed. err = GetEndPointManager().NewEndPoint(&mPreAllocatedConnectEP); } if (err == CHIP_NO_ERROR) @@ -446,6 +450,11 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) } } + if (mPreAllocatedConnectEP) + { + // If the Listen EndPoint has a pre-allocated connect EndPoint, release it for the Retain() in the constructor + mPreAllocatedConnectEP->Free(); + } if (mState == State::kClosed) { mUnackedLength = 0; @@ -816,7 +825,8 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p conEP = static_cast(listenEP->mPreAllocatedConnectEP); if (conEP == nullptr) { - // The listen endpoint receives a new incomming connection before it pre-allocates a new connection endpoint. + // The listen endpoint received a new incoming connection before it had a chance to pre-allocates a new connection + // endpoint. err = CHIP_ERROR_BUSY; } } @@ -849,8 +859,18 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p // Post a callback to the HandleConnectionReceived() function, passing it the new end point. listenEP->Retain(); conEP->Retain(); + // Hand over the implied ref from constructing mPreAllocatedConnectEP to + // ongoing connection. + listenEP->mPreAllocatedConnectEP = nullptr; + err = lSystemLayer.ScheduleLambda([listenEP, conEP] { listenEP->HandleIncomingConnection(conEP); + // Pre-allocate another endpoint for next connection if current connection is established + CHIP_ERROR error = listenEP->GetEndPointManager().NewEndPoint(&listenEP->mPreAllocatedConnectEP); + if (error != CHIP_NO_ERROR) + { + listenEP->HandleError(error); + } conEP->Release(); listenEP->Release(); }); @@ -861,24 +881,6 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p err = CHIP_ERROR_CONNECTION_ABORTED; conEP->Release(); // for the Retain() above } - else - { - // Pre-allocate another endpoint for next connection - listenEP->mPreAllocatedConnectEP = nullptr; - listenEP->Retain(); - err = lSystemLayer.ScheduleLambda([listenEP]() { - CHIP_ERROR error = listenEP->GetEndPointManager().NewEndPoint(&listenEP->mPreAllocatedConnectEP); - if (error != CHIP_NO_ERROR) - { - listenEP->HandleError(error); - } - listenEP->Release(); - }); - if (err != CHIP_NO_ERROR) - { - listenEP->Release(); - } - } } // Otherwise, there was an error accepting the connection, so post a callback to the HandleError function. From 170090434e60cfdee3ebe1206376b50b458fbdad Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Mon, 17 Feb 2025 17:15:05 +0800 Subject: [PATCH 28/29] typo fix --- src/inet/TCPEndPointImplLwIP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index 228e6d139d09c9..a89ee8480e02eb 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -825,7 +825,7 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p conEP = static_cast(listenEP->mPreAllocatedConnectEP); if (conEP == nullptr) { - // The listen endpoint received a new incoming connection before it had a chance to pre-allocates a new connection + // The listen endpoint received a new incoming connection before it had a chance to pre-allocate a new connection // endpoint. err = CHIP_ERROR_BUSY; } From 3019a97e7128add8a03ce55074e46f424e56026d Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Thu, 20 Feb 2025 10:24:11 +0800 Subject: [PATCH 29/29] Update src/inet/TCPEndPointImplLwIP.cpp Co-authored-by: Boris Zbarsky --- src/inet/TCPEndPointImplLwIP.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp index a89ee8480e02eb..af543c2ce90c12 100644 --- a/src/inet/TCPEndPointImplLwIP.cpp +++ b/src/inet/TCPEndPointImplLwIP.cpp @@ -454,6 +454,7 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState) { // If the Listen EndPoint has a pre-allocated connect EndPoint, release it for the Retain() in the constructor mPreAllocatedConnectEP->Free(); + mPreAllocatedConnectEP = nullptr; } if (mState == State::kClosed) {