fix(Core/Threading): Refactored LockedQueue / MPSCQueue - Improve thread safety, performance, and memory management

[nl-saw]

Changes Proposed:

Refactored LockedQueue class:

Thread-Safety Improvements: use std::atomic for the cancellation flag, ensuring proper atomic operations and synchronization.

Performance: By using move semantics in add() and next(), it avoids unnecessary copies, which is more efficient.

Simplified Locking: It uses std::lock_guardstd::mutex consistently for thread safety, reducing the chance of errors related to manual locking and unlocking.

Const-Correctness: It marks methods that don’t modify the object as const.

No Need for Manual Unlocking in Peek: The second version’s peek() method avoids the complexity of manually unlocking the mutex, which makes it easier to use safely.

Refactored MPSCQueue class:

Stricter Memory Ordering: Ensures stricter memory order semantics are used consistently using std::memory_order_acquire and std::memory_order_release.

Complete Node Deletion:
avoiding potential memory leaks or dangling pointers.

Improved Synchronization on Initialization.

This PR proposes changes to:

• Core (units, players, creatures, game systems).
• Scripts (bosses, spell scripts, creature scripts).
• Database (SAI, creatures, etc).

Issues Addressed:

• Closes

SOURCE:

The changes have been validated through:

• Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
• Sniffs (remember to share them with the open source community!)
• Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
• The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found

Tests Performed:

This PR has been:

• Tested in-game by the author.
• Tested in-game by other community members/someone else other than the author/has been live on production servers.
• This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

No functional changes have been made. The changes primarily involve code refactoring aimed at potential performance improvements, with enhanced code structure and more efficient memory handling for greater robustness and stability.
There should be no issues or deficiencies resulting from these changes.

• This pull request can be tested by following the reproduction steps provided in the linked issue
• This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.

Known Issues and TODO List:

• [ ]
• [ ]

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

[nl-saw]

Note: I am not 100% sure if it is really required to replace the memory_order_relaxed within the Dmitry Vyukov's lock free MPSC queue, so that can be debatable due to its single consumer concept.

[Winfidonarleyan]

Looks like, anyone test this?

[blinkysc]

Been running on mine for a few days (helped with testing pre PR). Been real smooth

[nl-saw]

Personally, I have tested this on Linux, a friend on Windows.

What has been tested:

• General gameplay
• Dungeons / Raids
• PvP / Arena / BG
• with 8500 bots scattered across the maps
• in a session with at least 19+ hours uptime
• using multiple wow clients

Nothing out of the ordinary has been observed.

Connected players: 1. Characters in world: 8501.
Connection peak: 6.
Server uptime: 19 hour(s) 30 second(s)
Update time diff: 35ms. Last 500 diffs summary:
|- Mean: 50ms
|- Median: 42ms
|- Percentiles (95, 99, max): 85ms, 113ms, 117ms

[sudlud]

@walkline @Takenbacon

[Takenbacon]

I did glance at it and looks okay to me. The only small hesitation I have is the change to stricter ordering on the MPSC queue, not sure if it's really necessary or if it may or may not have any impact on throughput.

[nl-saw]

I did glance at it and looks okay to me. The only small hesitation I have is the change to stricter ordering on the MPSC queue, not sure if it's really necessary or if it may or may not have any impact on throughput.

This is where I was somewhat hesitant as well, yet I haven't noticed an performance impact and overall it just sounds safer to me.