[tree] Fix bulk read inflating the reported branch size

[guitargeek]

Calling GetBulkRead().GetBulkEntries()/GetEntriesSerialized() on a branch increased its reported Total Size by 4 bytes on every call, so the size was not invariant with respect to the API used to read the branch.

Root cause: a bulk read loads a basket into fBaskets via TObjArray::AddAt (TBranch::GetBasketImpl), which raises the array's fLast. The bulk-IO path then disassociates that basket from the array with a plain slot assignment, fBaskets[fReadBasket] = nullptr. That is not the inverse of AddAt: the non-const TObjArray::operator[] does
fLast = std::max(j, GetAbsLast()), so writing the null actually pins fLast at fReadBasket rather than lowering it. The array is then streamed (by GetTotalSize()/Print(), and on a subsequent TTree::Write()) with one extra trailing slot, and TObjArray::Streamer emits an additional 4-byte null-pointer marker for it.

Fix: drop the basket with fBaskets.RemoveAt(fReadBasket) at the three bulk-IO disassociation sites (GetBasketAndFirst, GetBulkEntries, GetEntriesSerialized). RemoveAt is the proper inverse of the AddAt done when the basket was loaded: it clears the slot and walks fLast back down past trailing nulls, matching what DropBaskets already does.

Notes for review:

• Fix location. The alternative is to make TObjArray::Streamer / GetTotalSize tolerant of trailing null slots. Fixing the read path was chosen instead so that a bulk read leaves no observable state change at all, which is what the issue asks for, and it is narrower than teaching the streamer to compact.

• Performance. Bulk IO is a hot path, but this adds no meaningful cost: the previous operator[] assignment already acquired gCoreMutex (a conditional read guard, a no-op unless thread-safety is enabled) and already called Changed(). The only delta is read-guard -> write-guard.

• Scope. Other sites null a slot the same inflating way and share the latent bug (the fBaskets[i] = nullptr in the flush path, and the fBaskets.AddAt(nullptr, idx) calls in the basket-unload paths). They are intentionally left untouched here: they are unrelated code paths not exercised by this issue, and are better addressed separately.

• RemoveAt is a no-op on an already-null slot, so there is no double-remove hazard when GetBulkEntries runs after GetBasketAndFirst cleared the slot. The disassociated basket is not leaked; it is still parked in fExtraBasket.

Adds a regression test (BulkApiTest.SizeInvariantAfterBulkRead) asserting GetTotalSize() is invariant across a bulk read; it fails without the fix.

Closes #8961

🤖 Done with the help of AI.

[github-actions]

Test Results

    23 files      23 suites   3d 15h 40m 51s ⏱️
 3 882 tests  3 881 ✅ 0 💤 1 ❌
79 963 runs  79 954 ✅ 8 💤 1 ❌

For more details on these failures, see this check.

Results for commit bf8a530.

♻️ This comment has been updated with latest results.