fix(command): fix static initialization crash on Kunpeng/Kylin platform

[sryanyuan]

Problem:

We've identified a critical startup crash specific to Kunpeng processors (aarch64) running Kylin OS v10 with GCC 12.5.0. The root cause is a static initialization order issue where:

• Static command registration objects (e.g., RegisterToCommandTable in command files) initialize before CommandTable's static containers
• This leads to accessing uninitialized containers during startup:

CommandTable::commands[attr.name] = ...;  // ❌ Accesses uninitialized map

• On Kunpeng/Kylin/GCC12.5, this consistently manifests as:
  • Segmentation faults during static initialization
  • 100% failure rate in cold starts

Solution:

Implement the "construct on first use" pattern to guarantee safe initialization.

Key Changes:

• Converted all static members in CommandTable to accessor functions
• Updated all references to use function-call
• Maintained identical public interface while fixing initialization guarantees

[PragmaTwice]

This will introduce a mutex to every call, which is exactly the reason why I avoid to use static local variables.

[sryanyuan]

I think there is a small misunderstanding here.

Using a function-local static does not imply taking a lock on every call. With C++11 thread-safe statics, synchronization is only needed during the first initialization. After initialization, the common case is just a guard check, which is typically a load plus a branch, not a contended lock operation.

Also, after the object has been initialized, that guard is effectively read-only. So in practice this should not create the kind of MESI ping-pong that we usually worry about with frequently written shared state. The steady-state overhead is much closer to an extra branch than to repeated locking.

In this code path, the cost is also likely dominated by the following map lookup and string comparisons, so the incremental overhead of the guard check should be negligible relative to the rest of the operation.

The reason for this change is to avoid the static initialization order issue across translation units, which is exactly what caused the startup crash here. So this trades a very small steady-state check for correctness and deterministic initialization.

[PragmaTwice]

I feel that I'm chatting with Claude code instead of a real person 😅

[PragmaTwice]

Static command registration objects (e.g., RegisterToCommandTable in command files) initialize before CommandTable's static containers

This should not happen. Check related wording in the standard.

After all static initialization is completed, dynamic initialization of non-local variables occurs in the following situations:

1. Unordered dynamic initialization, which applies only to (static/thread-local) class template static data members and variable templates(since C++14) that aren't explicitly specialized. Initialization of such static variables is indeterminately sequenced with respect to all other dynamic initialization except if the program starts a thread before a variable is initialized, in which case its initialization is unsequenced (since C++17). Initialization of such thread-local variables is unsequenced with respect to all other dynamic initialization.
2. Partially-ordered dynamic initialization, which applies to all inline variables that are not an implicitly or explicitly instantiated specialization. If a partially-ordered V is defined before ordered or partially-ordered W in every translation unit, the initialization of V is sequenced before the initialization of W (or happens-before, if the program starts a thread). (since C++17)
3. Ordered dynamic initialization, which applies to all other non-local variables: within a single translation unit, initialization of these variables is always sequenced in exact order their definitions appear in the source code. Initialization of static variables in different translation units is indeterminately sequenced. Initialization of thread-local variables in different translation units is unsequenced.

[sryanyuan]

Static command registration objects (e.g., RegisterToCommandTable in command files) initialize before CommandTable's static containers

This should not happen. Check related wording in the standard.

After all static initialization is completed, dynamic initialization of non-local variables occurs in the following situations:

1. Unordered dynamic initialization, which applies only to (static/thread-local) class template static data members and variable templates(since C++14) that aren't explicitly specialized. Initialization of such static variables is indeterminately sequenced with respect to all other dynamic initialization except if the program starts a thread before a variable is initialized, in which case its initialization is unsequenced (since C++17). Initialization of such thread-local variables is unsequenced with respect to all other dynamic initialization.
2. Partially-ordered dynamic initialization, which applies to all inline variables that are not an implicitly or explicitly instantiated specialization. If a partially-ordered V is defined before ordered or partially-ordered W in every translation unit, the initialization of V is sequenced before the initialization of W (or happens-before, if the program starts a thread). (since C++17)
3. Ordered dynamic initialization, which applies to all other non-local variables: within a single translation unit, initialization of these variables is always sequenced in exact order their definitions appear in the source code. Initialization of static variables in different translation units is indeterminately sequenced. Initialization of thread-local variables in different translation units is unsequenced.

I agree that, based on the standard wording, this should not happen in theory.

What makes this tricky is that we did hit a real startup crash on KylinOS + GCC 12.5.0 in this path, and the function-local static version fixes it consistently. So at the moment my working assumption is that we are dealing with either a toolchain-specific issue or some implementation-sensitive behavior, rather than a contradiction of the standard itself.

To verify that, we will also build a minimal multi-translation-unit reproducer and use it to determine whether the root cause is in the toolchain, the initialization model, or some interaction specific to our codebase.

[jihuayu]

If this is inconsistent with the standard, could it be an issue with the upstream GCC? Would using a different compiler or a newer version resolve the problem?

I don't think it's acceptable to introduce additional costs for all platforms just to fix issues with certain upstream components.

[sryanyuan]

If this is inconsistent with the standard, could it be an issue with the upstream GCC? Would using a different compiler or a newer version resolve the problem?

I don't think it's acceptable to introduce additional costs for all platforms just to fix issues with certain upstream components.

I've reproduced the crash consistently under optimized builds (-O2/-O3) across GCC 12.5.0 and 13.4.0. The failure occurs specifically during static initialization in:

redis::RegisterToCommandTable::RegisterToCommandTable(  
    redis::CommandCategory,  
    std::initializer_list<redis::CommandAttributes>  
)  

The crash disappears when i disable optimizations only for commander.cc. I'm conducting further analysis to identify the root cause.

[sryanyuan]

@PragmaTwice @jihuayu

I've confirmed that when LTO is enabled, the compiler reorders the construction of static variables, which causes crashes on ARM64 with GCC 12.5.0/13.4.0. As a temporary solution, I'll disable LTO for the binary build. Thank you for your patient guidance throughout this issue.

[PragmaTwice]

@PragmaTwice @jihuayu

I've confirmed that when LTO is enabled, the compiler reorders the construction of static variables, which causes crashes on ARM64 with GCC 12.5.0/13.4.0. As a temporary solution, I'll disable LTO for the binary build. Thank you for your patient guidance throughout this issue.

Hmm I think we can disable LTO by default in CMake to prevent such issues.

[sryanyuan]

@PragmaTwice @jihuayu
I've confirmed that when LTO is enabled, the compiler reorders the construction of static variables, which causes crashes on ARM64 with GCC 12.5.0/13.4.0. As a temporary solution, I'll disable LTO for the binary build. Thank you for your patient guidance throughout this issue.

Hmm I think we can disable LTO by default in CMake to prevent such issues.

I will add an option in CMake to allow manually enabling LTO when I have some free time.

[PragmaTwice]

kvrocks/CMakeLists.txt

Line 33 in 5f6d7b2

option(ENABLE_LTO "enable link-time optimization" ON)

Just need to toggle ON to OFF.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
66.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud