From ae8a002a3e51fc35495e6c76a2ef97f3cf00364f Mon Sep 17 00:00:00 2001 From: Martin Ecker Date: Thu, 3 Sep 2026 14:51:38 -0700 Subject: [PATCH] Make the texture send queue own its packet copies Change TexturePendingServerAdd() to clone each CmdTexture before adding it to the send queue. This keeps objects in mTrackedTextures separate from packets being used by the communication thread. Update TextureTrackingRem() to queue a Destroy copy and immediately remove and free the tracked Create object. Free temporary managed-texture Update objects after their queue copies are created. Make Communications_Outgoing_Textures() release every queued texture packet after it is sent. Add Communications_ReleaseTexturePackets() to free pending and in-flight texture packets when Communications_Loop() disconnects or ClientInfo is destroyed. Stop accepting texture packets after mbDisconnectPending is set, clear packets from the old connection, and remove older pending packets when a Create or Destroy supersedes them. Walk mTrackedTextures backward when TextureTrackingRem() can erase entries. Update ClientInfo::TextureTrackingClear() to preserve texture IDs supplied by legacy ImGui renderers and generate an ID only when the texture does not already have one. These changes prevent texture packets from being changed or freed while they are being sent. They also prevent stale Updates from reaching a new server before their Creates and avoid leaking packets after an interrupted connection. --- Code/Client/Private/NetImgui_Client.cpp | 151 ++++++++++++++++++------ Code/Client/Private/NetImgui_Client.h | 2 +- 2 files changed, 115 insertions(+), 38 deletions(-) diff --git a/Code/Client/Private/NetImgui_Client.cpp b/Code/Client/Private/NetImgui_Client.cpp index 2b4600f..c214d00 100644 --- a/Code/Client/Private/NetImgui_Client.cpp +++ b/Code/Client/Private/NetImgui_Client.cpp @@ -156,11 +156,30 @@ void Communications_Outgoing_Textures(ClientInfo& client) client.mPendingTextures = client.mPendingTextures->mpNext; pPendingTexture->mpNext = nullptr; client.mPendingSend.pCommand = pPendingTexture; - client.mPendingSend.bAutoFree = false; // free handled by main update thread + client.mPendingSend.bAutoFree = true; } } } +static void Communications_ReleaseTexturePackets(ClientInfo& client) +{ + std::lock_guard guard(client.mPendingTexturesLock); + while( client.mPendingTextures ) + { + CmdTexture* pPendingTexture = client.mPendingTextures; + client.mPendingTextures = pPendingTexture->mpNext; + netImguiDelete(pPendingTexture); + } + + if( client.mPendingSend.pCommand && client.mPendingSend.pCommand->mType == CmdHeader::eCommands::Texture ) + { + IM_ASSERT(client.mPendingSend.bAutoFree); + CmdTexture* pPendingTexture = static_cast(client.mPendingSend.pCommand); + netImguiDelete(pPendingTexture); + client.mPendingSend = PendingCom(); + } +} + //================================================================================================= // OUTCOM: BACKGROUND // Transmit the current client background settings @@ -406,6 +425,7 @@ bool Communications_Initialize(ClientInfo& client) while( client.IsConnected() ); } + client.mbDisconnectPending = false; client.mpSocketComs = pNewComSocket; // Take ownerhip of socket client.mBGSettingSent.mTextureId = client.mBGSetting.mTextureId-1u; // Force sending the Background settings (by making different than current settings) client.mFrameIndex = 0; @@ -442,6 +462,10 @@ void Communications_Loop(void* pClientVoid) Communications_Incoming(*pClient); } + // Drop every texture packet tied to the old server before a new connection can become visible. + // This also releases an independently allocated packet when a send was interrupted. + Communications_ReleaseTexturePackets(*pClient); + Network::SocketInfo* pSocket = pClient->mpSocketComs.exchange(nullptr); if (pSocket){ NetImgui::Internal::Network::Disconnect(pSocket); @@ -536,6 +560,9 @@ ClientInfo::~ClientInfo() { ContextRemoveHooks(); + // Pending and in-flight texture packets are independent copies owned by the send path. + Communications_ReleaseTexturePackets(*this); + // Free all tracked textures for(auto cmdTexture : mTrackedTextures){ netImguiDelete(cmdTexture); @@ -703,9 +730,12 @@ void ClientInfo::TextureTrackingClear() { if (TexData->Status == ImTextureStatus_WantCreate ) { - IM_ASSERT(TexData->TexID == ImTextureID_Invalid && TexData->BackendUserData == nullptr); - static ImTextureID sUniqueID(1); - TexData->SetTexID(static_cast(sUniqueID++)); + IM_ASSERT(TexData->BackendUserData == nullptr); + if( TexData->TexID == ImTextureID_Invalid ) + { + static ImTextureID sUniqueID(1); + TexData->SetTexID(static_cast(sUniqueID++)); + } TexData->SetStatus(ImTextureStatus_OK); } else if (TexData->Status == ImTextureStatus_WantUpdates) @@ -724,8 +754,10 @@ void ClientInfo::TextureTrackingClear() // they will be resent on reconnect if( !IsConnected() && mDearImguiTextureCount > 0 ) { - for(auto pCmdTexture : mTrackedTextures ) + // TextureTrackingRem removes disconnected entries immediately, so iterate backwards. + for( int i = mTrackedTextures.Size - 1; i >= 0; --i ) { + CmdTexture* pCmdTexture = mTrackedTextures[i]; if( pCmdTexture->mIsDearImGuiManaged ) { mbTrackedTexturesPending |= TextureTrackingRem(pCmdTexture->mTextureClientID); @@ -801,8 +833,8 @@ void ClientInfo::TextureTrackingUpdate(bool bResendAll) pCmdTexture->mUpdatable = true; pCmdTexture->mIsDearImGuiManaged = true; pCmdTexture->mpTextureData.ToOffset(); - TexturePendingServerAdd(*pCmdTexture); // Request texture to be sent over to Server - mbTrackedTexturesPending = true; + TexturePendingServerAdd(*pCmdTexture); + netImguiDelete(pCmdTexture); } } } @@ -816,8 +848,10 @@ void ClientInfo::TextureTrackingUpdate(bool bResendAll) //------------------------------------------------------------------------ if( mDearImguiTextureCount != Textures.Size ) { - for(auto pCmdTexture : mTrackedTextures ) + // TextureTrackingRem uses erase-swap, so walk backwards to keep the remaining indices valid. + for( int trackedIndex = mTrackedTextures.Size - 1; trackedIndex >= 0; --trackedIndex ) { + CmdTexture* pCmdTexture = mTrackedTextures[trackedIndex]; if( pCmdTexture->mIsDearImGuiManaged ) { bool bFound(false); @@ -895,56 +929,99 @@ bool ClientInfo::TextureTrackingAdd(CmdTexture& cmdTexture) bool ClientInfo::TextureTrackingRem(ClientTextureID clientTextureID) { - // If texture has been sent to server, re-purpose existing command - // as a 'destroy' and re-send it to server for(int i(0); imTextureClientID == clientTextureID && pCmdTexture->mStatus == CmdTexture::eType::Create ) { - pCmdTexture->mSent = false; - pCmdTexture->mStatus = CmdTexture::eType::Destroy; // Re-purpose create cmd to destroy the texture - TexturePendingServerAdd(*pCmdTexture); - - // Remove item from our list + // The send queue receives its own copy, so this tracked object cannot be in use by the communication thread. + pCmdTexture->mStatus = CmdTexture::eType::Destroy; mDearImguiTextureCount -= pCmdTexture->mIsDearImGuiManaged ? 1 : 0; + TexturePendingServerAdd(*pCmdTexture); mTrackedTextures[i] = mTrackedTextures.back(); mTrackedTextures.pop_back(); + netImguiDelete(pCmdTexture); return true; } } return false; } -void ClientInfo::TexturePendingServerAdd(CmdTexture& cmdTexture) +static CmdTexture* TextureCmdCloneForQueue(const CmdTexture& cmdTexture) +{ + if( cmdTexture.mSize < sizeof(CmdTexture) ) + { + IM_ASSERT(false); + return nullptr; + } + + CmdTexture* pQueuedTexture = netImguiSizedNew(cmdTexture.mSize); + if( pQueuedTexture == nullptr ) + { + return nullptr; + } + + *pQueuedTexture = cmdTexture; + pQueuedTexture->mSent = false; + pQueuedTexture->mpNext = nullptr; + + const size_t textureDataSize = cmdTexture.mSize - sizeof(CmdTexture); + if( textureDataSize > 0 ) + { + const uint8_t* pTextureData = cmdTexture.mpTextureData.IsOffset() + ? reinterpret_cast(&cmdTexture.mpTextureData) + cmdTexture.mpTextureData.GetOff() + : cmdTexture.mpTextureData.Get(); + memcpy(&pQueuedTexture[1], pTextureData, textureDataSize); + } + pQueuedTexture->mpTextureData.SetPtr(reinterpret_cast(&pQueuedTexture[1])); + pQueuedTexture->mpTextureData.ToOffset(); + return pQueuedTexture; +} + +bool ClientInfo::TexturePendingServerAdd(const CmdTexture& cmdTexture) { std::lock_guard guard(mPendingTexturesLock); - if( IsConnected() ) + const bool canQueue = IsConnected() && !mbDisconnectPending; + if( !canQueue ) { - // Find last added entry - CmdTexture** ppNextTexture = &mPendingTextures; - CmdTexture* pendingTexture = mPendingTextures; - while( pendingTexture != nullptr ) + // Every queued packet belongs to the old connection, not just packets for this texture. + while( mPendingTextures ) { - // Remove all unprocessed texture commands with same id - // (only need the latest action for create/destroy, but can have multiple update queued) - if( cmdTexture.mStatus != CmdTexture::eType::Update && - cmdTexture.mSent == false && - cmdTexture.mTextureClientID == pendingTexture->mTextureClientID ) - { - // Mark as sent and un-needed (which gets it removed from tracking array and deleted later) - pendingTexture->mSent = true; - pendingTexture->mStatus = CmdTexture::eType::Destroy; - *ppNextTexture = pendingTexture->mpNext; - } - ppNextTexture = &pendingTexture->mpNext; - pendingTexture = pendingTexture->mpNext; + CmdTexture* pPendingTexture = mPendingTextures; + mPendingTextures = pPendingTexture->mpNext; + netImguiDelete(pPendingTexture); } + return false; + } - // Add as last element and ready to be sent - cmdTexture.mSent = false; - *ppNextTexture = &cmdTexture; + CmdTexture* pQueuedTexture = TextureCmdCloneForQueue(cmdTexture); + if( pQueuedTexture == nullptr ) + { + return false; + } + + // A Create or Destroy supersedes every older pending command for the same texture. + CmdTexture** ppNextTexture = &mPendingTextures; + CmdTexture* pPendingTexture = mPendingTextures; + while( pPendingTexture != nullptr ) + { + if( cmdTexture.mStatus != CmdTexture::eType::Update && + cmdTexture.mTextureClientID == pPendingTexture->mTextureClientID ) + { + CmdTexture* pRemovedTexture = pPendingTexture; + *ppNextTexture = pRemovedTexture->mpNext; + pPendingTexture = *ppNextTexture; + pRemovedTexture->mpNext = nullptr; + netImguiDelete(pRemovedTexture); + continue; + } + + ppNextTexture = &pPendingTexture->mpNext; + pPendingTexture = pPendingTexture->mpNext; } + + *ppNextTexture = pQueuedTexture; + return true; } //================================================================================================= diff --git a/Code/Client/Private/NetImgui_Client.h b/Code/Client/Private/NetImgui_Client.h index 738b2ff..eb475d6 100644 --- a/Code/Client/Private/NetImgui_Client.h +++ b/Code/Client/Private/NetImgui_Client.h @@ -79,7 +79,7 @@ struct ClientInfo void TextureTrackingUpdate(bool bResendAll=false); // Process Backend ImGui textures CmdTexture* TextureCmdAllocate(ClientTextureID clientTexID, uint16_t width, uint16_t height, eTexFormat format, uint32_t& dataSizeInOut); - void TexturePendingServerAdd(CmdTexture& cmdTexture); // Add CmdTexture to list of command waiting for send off to Server + bool TexturePendingServerAdd(const CmdTexture& cmdTexture); // Add an owned copy to the send queue. void ProcessDrawData(const ImDrawData* pDearImguiData, ImGuiMouseCursor mouseCursor);