Server app crashes when more than 64 textures are synchonized by network

[lemantisee]

Describe the bug First for all, thank you for a greate library!

Server app crashes when more than 64 textures are synchonized by network.

I think the problem can be in undefined behavior between thread which read and thread which write texures to Client::mpPendingTextures buffer. Client::ReceiveTexture method can write texture to same array cell which is reading now in Client::ProcessPendingTextures method. This is"data race" and can crash the server app. There is no any multithread synchronization (mutex, conditional values, etc.) between this two calls. Only read/write operation on index is synced by atomic values.

If I change Client::mpPendingTextures to 128 size the problem is gone.

In my project I reimplement this logic in a way to use std::queue to push/pop textures in different threads and synchonize this calls wtith std::mutex (conditional values) and checking max size of queue (64). Made PR for this change #27 .

To Reproduce Steps to reproduce the behavior:

1. Sync more than 64 textures between client and server

Expected behavior All textures synced correctlly without undefined behavior

Desktop (please complete the following information):

• OS: Windows 10
• Version 1.75

[sammyfreg]

Thanks! I'm glad to see the code was clear enough for you to find and fix the issue.

I have been avoiding using std containers / mutex as much as possible, keeping performances up and memory usage down, but this is not as critical in the Server app. I mostly cared about minimizing impact on client code. I was not expecting a high amount of textures (rookie mistake :) ), so this had not been tested as much. The texture lookup on the server is not ideal either, since I just iterate the vector for every drawcall, looking for the textureid.

I will take a look this weekend, might take your PR as is, or maybe just do what I did on the client side, use a spin lock in rare case where we are about to overflow the ring buffer.

[lemantisee]

Thank you for quick reply! Is there any code conventions? I have idea to next PR and want write code as more closer to original sources :)

[sammyfreg]

I have not formalized a coding convention for the project.

I more or less follow this I usually prefix class variable with 'm', pointer with 'p', bool with 'b' and constant with 'k'. I use camel case, with first letter lowercase for function parameters, uppercase start for local variables. I usually try to use typed class enums. Defines are all uppercase With a series of assignments, I align them by the equal operator. Warning level is set to 'Wall' so I fix all compiling warnings.

If something stick out in your PR code (coding style difference), I will let you know.

[sammyfreg]

Note: you can search for mTexturesPendingCreated to see how did my spin lock on the client side, avoiding a mutex/dynamic mem alloc. (NetImgui_Api.cpp : 323)

[lemantisee]

28 fix with client side spin lock logic

[sammyfreg]

PR 28 fixed the issue