#6172 Crash on recordResultCode

[akleshchev]

mResutCodes gets modified from different threads, mutex it.

[Copilot]

Pull request overview

This PR addresses a crash in HTTPStats::recordResultCode() by adding mutual exclusion around the shared result-code map, which is accessed from multiple threads in the HTTP subsystem.

Changes:

• Added a mutex to protect access to the result-code statistics map.
• Wrapped recordResultCode(), resetStats(), and dumpStats() result-code map access in std::lock_guard.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
indra/llcorehttp/httpstats.h	Adds a mutex member and annotates recordResultCode() threading context.
indra/llcorehttp/httpstats.cpp	Applies locking around result-code map mutation/iteration in reset/record/dump paths.

Suppressed comments (1)

indra/llcorehttp/httpstats.cpp:105

• dumpStats() reads mDataUp/mDataDown/mRequests without synchronization while those can be updated from other threads. If using mResultCodesMutex to guard all stats updates, snapshot these values (and a copy of the result-code map) under the lock, then format output after releasing the lock to minimize blocking the HTTP thread.

    out << "Result Codes:" << std::endl << "--- -----" << std::endl;
    {
        std::lock_guard<std::mutex> lock(mResultCodesMutex);
        for (std::map<S32, S32>::iterator it = mResutCodes.begin(); it != mResutCodes.end(); ++it)
        {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[akleshchev]

Makes sense. I focused on recordResultCode, since it's the only one called by stageAfterCompletion, but in this case it's cheap to cover more.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

indra/llcorehttp/httpstats.h:62

• recordHTTPRequest() increments mRequests without holding mStatsMutex, while resetStats()/dumpStats() now access mRequests under that mutex. This leaves a remaining data race (and can still crash/produce corrupted stats) when HttpRequest objects are created concurrently with resetStats()/dumpStats(). Guard the increment with the same mutex (or make mRequests atomic and adjust callers accordingly).

        void    recordHTTPRequest() { ++mRequests; }

        void    recordResultCode(S32 code); // http thread