dispatcher bug

[patrykq112]

Priority

High

Area

• [ ] Data
• [X] Source
• [ ] Docker
• [ ] Other

What happened?

@mehah

event dispatcher crashes the client on computers with a weaker processor. It calls the "getThreadTask()" function which refers to "m_threads", if "getThreadId()" goes beyond the reserved space of "m_threads" then a crash occurs. The problem is in "m_threads.reserve(g_asyncDispatcher.get_thread_count() + 1);" if I changed it to "m_threads.reserve(1000)" for test then the crashes stopped. We need a better solution for this.

What OS are you seeing the problem on?

Linux, Windows, MacOS

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[patrykq112]

my solution:

int16_t EventDispatcher::getThreadId() {
    static std::atomic_int16_t lastId = -1;
    thread_local static int16_t id = -1;

    if (id == -1) {
        if (lastId.load() <= g_asyncDispatcher.get_thread_count()) {
            lastId.fetch_add(1);
        } else {
            lastId.store(0);
        }
        id = lastId.load();
    }

    return id;
};

[mehah]

@patrykq112

How many cores does the PC that is having the problem have?

try: m_threads.reserve(g_asyncDispatcher.get_thread_count() + 2);

[mehah]

my solution:

int16_t EventDispatcher::getThreadId() {
    static std::atomic_int16_t lastId = -1;
    thread_local static int16_t id = -1;

    if (id == -1) {
        if (lastId.load() <= g_asyncDispatcher.get_thread_count()) {
            lastId.fetch_add(1);
        } else {
            lastId.store(0);
        }
        id = lastId.load();
    }

    return id;
};

You can't do that, this is a method to generate unique ids for each existing thread, this way, threads that exist beyond the thread_pool will have repeated ids.

[patrykq112]

my solution:

int16_t EventDispatcher::getThreadId() {
    static std::atomic_int16_t lastId = -1;
    thread_local static int16_t id = -1;

    if (id == -1) {
        if (lastId.load() <= g_asyncDispatcher.get_thread_count()) {
            lastId.fetch_add(1);
        } else {
            lastId.store(0);
        }
        id = lastId.load();
    }

    return id;
};

You can't do that, this is a method to generate unique ids for each existing thread, this way, threads that exist beyond the thread_pool will have repeated ids.

@patrykq112

How many cores does the PC that is having the problem have?

try: m_threads.reserve(g_asyncDispatcher.get_thread_count() + 2);

players with old processors have reported this problem, for me this problem only occurs on Android

[mehah]

@patrykq112 It's strange, I need to know the index it is trying to access and the size of m_threads to know the ideal value, try putting +2;

[patrykq112]

@patrykq112 It's strange, I need to know the index it is trying to access and the size of m_threads to know the ideal value, try putting +2;

on the device where the crashes occur, g_asyncDispatcher.get_thread_count() returns 7

[mehah]

@patrykq112 And these cpu have how many cores?

[patrykq112]

@patrykq112 And these cpu have how many cores?

I test on the Snapdragon 7s Gen 2 processor, it has 8 cores.

crashes occur if space for "m_threads" is reserved below 9.

[patrykq112]

I changed the space reservation for "m_threads" to: m_threads.reserve(std::max<int>(9, g_asyncDispatcher.get_thread_count())); and it's good

[mehah]

@patrykq112 We can't work this way, if a PC has 2 cores, you will instantiate 9 threads unnecessarily.

try m_threads.reserve(g_asyncDispatcher.get_thread_count() + 2);

[mehah]

I will rewrite this method to instantiate on demand.

[mehah]

@patrykq112 test https://github.com/mehah/otclient/pull/770

[patrykq112]

@patrykq112 test #770

@mehah works, no longer crashes