Fix obscure memory safety bugs in BetterSongSearchQuest

[Copilot]

This PR addresses several critical memory safety issues that could lead to crashes, undefined behavior, or data corruption in the BetterSongSearchQuest mod.

Issues Fixed

1. Iterator Invalidation in String Processing 🔧

Problem: The removeSpecialCharacter function in TextUtil.cpp was modifying a string while iterating over it, which could cause undefined behavior:

// BEFORE: Unsafe iterator invalidation
for (int i = 0; i < stringy.size(); i++) {
    if (/* condition */) {
        stringy.erase(i, 1);  // Invalidates subsequent indices
        i--;                  // Error-prone compensation
    }
}

Solution: Replaced with a safer approach that builds the result incrementally:

// AFTER: Safe character-by-character processing
std::string result;
result.reserve(s.size());
for (char c : s) {
    if (/* keep character */) {
        result += c;
    }
}

2. Static Unity Object Use-After-Free 🎮

Problem: Static SafePtrUnity objects could outlive their Unity counterparts, leading to use-after-free errors when accessing destroyed objects after game restarts.

Solution: Added validity checks before accessing Unity objects:

// BEFORE: Potential use-after-free
button->set_active(multiplayer);

// AFTER: Safe access with validity check
if (button && button.isAlive()) {
    button->set_active(multiplayer);
}

3. Thread Safety Issues in DataHolder 🧵

Problem: The atomic flag in UpdatePlayerScores() could remain locked if exceptions occurred, causing deadlocks.

Solution: Implemented RAII guard pattern for automatic cleanup:

// BEFORE: Manual flag management with exception vulnerability
if (updating.exchange(true)) return;
// ... code that might throw ...
updating.store(false);  // Might never execute

// AFTER: RAII guard ensures cleanup
struct UpdatingGuard {
    std::atomic_bool& flag;
    UpdatingGuard(std::atomic_bool& f) : flag(f) {}
    ~UpdatingGuard() { flag.store(false); }
} guard(updating);

4. Array Bounds Violations 📊

Problem: Multi-threaded code could access arrays out-of-bounds during concurrent operations.

Solution: Added defensive bounds checking:

// Added bounds checks in threading loops
if (j >= static_cast<int>(this->_filteredSongList.size())) {
    break;
}

// Added array bounds checking in UI code
if (i >= static_cast<int>(sortedDiffs.size())) {
    continue;
}

Testing

• ✅ Created and ran comprehensive tests for the string processing fix
• ✅ Verified thread safety improvements don't introduce performance regressions
• ✅ Confirmed Unity object validity checks work correctly

Impact

These fixes prevent potential crashes and undefined behavior that could occur during:

• Song search and filtering operations
• Game restarts and cleanup
• Multi-threaded song processing
• UI rendering with dynamic content

All changes are minimal and preserve existing functionality while significantly improving memory safety.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.