chronoxor / FastBinaryEncoding

Fast Binary Encoding is ultra fast and universal serialization solution for C++, C#, Go, Java, JavaScript, Kotlin, Python, Ruby, Swift
https://chronoxor.github.io/FastBinaryEncoding
MIT License
876 stars 90 forks source link

Watchdog timeout bug in x86 #80

Closed korllan closed 1 year ago

korllan commented 1 year ago

The request functions of the Client class receive a optional parameter size_t timeout.

Example: std::future<void> request(const ::proto::Ping& value, size_t timeout = 0);

Inside the function code we multiply this timeout by timeout * 1000000, since size_t is a macro that change the variable size based on x86 / x64 build, when compiling in x86 the timeout size is unsigned int, if we use a timeout bigger than ~4.3 seconds we have a arithmetic overflow on this multiply operation, causing the malfunction of the watchdog timeout.

Solution: change the function parameter to uint64_t or cast the multiply operation to uint64_t (uint64_t)timeout * 1000000.

Another question that I have about the watchdog is what the best way to run the watchdog? Currently I'm running it in a another thread with 50ms sleep between the calls to it, but I don't know if this imply in a performance decrease since the watchdog will be locking the mutex every 50ms.

It would be nice to have some documentation on the watchdog function, as it is not mentioned anywhere in the documentation (at least I didn't find it), I discovered it by looking at the code generated by FBE.

static DWORD WINAPI WatchdogEventsThread(CWsClient* WsClient)
{
    while (true)
    {
        try
        {
            WsClient->GetWsClient()->watchdog(FBE::utc());
        }
        catch (...)
        {
            LOG("Exception on watchdog thread.");
        }

        std::this_thread::sleep_for(std::chrono::milliseconds(50));
    }

    return 0;
}
korllan commented 1 year ago

@chronoxor can you explain more? what else would break on x86? I have a entire project made using FBE, the server is using NodeJS and the client is in C++ x86.

chronoxor commented 1 year ago

@korllan generated C++ FBE should work fine with x86, I missed with answer. Also I fixed the timeout type in generated request() methods. Please try FBE 1.14.2.0

chronoxor commented 1 year ago

We schedule a watchdog timeout client thread for one second:

const Timespan WATCHDOG_TIMEOUT = Timespan::seconds(1);