CNA-BUG-165: LeaderboardReader::Read, PageDown and PageUp never delete the LeaderboardAction their Begin call allocates

CNA snapshot 009d40f5  ·  Known Issues › Current bugs  ·  source links pinned to 009d40f5

The three synchronous Read overloads and PageDown/PageUp call their Begin*/End* pair and drop the heap-allocated IAsyncResult, leaking one LeaderboardAction per call, unlike Gamer::GetProfile and SignedInGamer::GetAchievements.

Identifier

CNA-BUG-165

Category

Bug

Subsystem

Networking & gamer services

Status

Open

Verified against

CNA 009d40f5 (009d40f5dd085c4e674d3479675fac84b12b3e0a)

Severity

Low (a triage suggestion, not a project priority)

Evidence basis

Source-verified: read at TARGET, not executed

Tests touching this area

Yes: see Current tests

Affected contract

Microsoft::Xna::Framework::GamerServices::LeaderboardReader::Read (all three overloads), PageDown() and PageUp(); the C routes cna_leaderboard_reader_page_down and cna_leaderboard_reader_page_up

Expected behaviour

Begin* returns a raw, caller-owned IAsyncResult*: the tests delete it after End*, and the other synchronous wrappers, Gamer::GetProfile and SignedInGamer::GetAchievements, delete theirs with the comment "caller-owned; FNA relies on GC". A synchronous wrapper is that caller and must free it.

Actual behaviour at TARGET

In LeaderboardReader.cpp, each Read overload returns EndRead(result), and PageDown()/PageUp() return after EndPageDown/EndPageUp, without deleting result. Every call leaks one LeaderboardAction: its callback, its state, an EventWaitHandle and, for Read, the moved-from reader kept in its std::optional. CNA's own C adapter records the defect: CnaCApiLeaderboards.cpp says the canonical Read overloads "never release the operation those create -- unlike Gamer::GetProfile" and performs BeginRead/EndRead itself; its page-down and page-up routes, however, call the leaking PageDown()/PageUp().

Source locations

• modules/gamer-services/src/Xna/LeaderboardReader.cpp — LeaderboardReader::Read overloads, PageDown and PageUp; CompleteReadEXT and CompletePageEXT allocate the action
• modules/gamer-services/src/Xna/Gamer.cpp — Gamer::GetProfile deletes its result
• modules/gamer-services/src/Xna/SignedInGamer.cpp — SignedInGamer::GetAchievements deletes its result
• modules/c-api/src/CnaCApiLeaderboards.cpp — ReadThroughOperation comment records the leak; the page routes call PageDown and PageUp
• modules/gamer-services/tests/Microsoft/Xna/Framework/GamerServices/GamerServicesGamerTests.cpp — LeaderboardReaderTest Begin cases delete their results

Evidence

Checked by reading LeaderboardReader.cpp, the other wrappers, the C adapter and the tests at 009d40f5; not executed and not measured under LeakSanitizer. LeaderboardAction is a translation-unit-private type with no instance counter, so no current test can observe the leak. The C adapter's comment is CNA's own independent record of the Read half.

Independently observed as a separate finding (merged): The synchronous wrappers call Begin*/End* and drop the heap-allocated IAsyncResult, leaking one object per call, unlike Gamer::GetProfile and SignedInGamer::GetAchievements.

Focused reproduction

// Illustrative; not compiled or run for this entry. Run under LeakSanitizer.
auto board = LeaderboardIdentity::Create(LeaderboardKey::BestScoreLifeTime);
for (int i = 0; i < 1000; ++i)
{
    LeaderboardReader page = LeaderboardReader::Read(board, 0, 10);   // leaks one action
}
// Expected: 1000 direct leaks allocated in CompleteReadEXT.

Current tests

The LeaderboardReaderTest cases in GamerServicesGamerTests.cpp call Read, PageDown and PageUp and check entries, paging and exceptions; the Begin* cases delete their results. None counts live actions.

Regression test

An instance counter on the action (the NetworkSessionAction::GetInstanceCountForTesting pattern) or a LeakSanitizer lane over LeaderboardReaderTest, asserting that no action survives Read, PageDown or PageUp; the fix is a std::unique_ptr around the result in the three wrappers.

Blast radius

Every synchronous leaderboard read and page turn, from C++ or through the C page routes: one small heap object per call, unbounded over a session that pages repeatedly. Begin*/End* callers that delete their result, and the C read routes, are unaffected.

Workaround

Use BeginRead/EndRead and BeginPageDown/EndPageDown (or PageUp) directly and delete the result after End.