Skip to content

Make SbPList growth exception safe - #740

Open
Dikluwe wants to merge 1 commit into
coin3d:masterfrom
Dikluwe:fix/sbplist-growth-exception-safety
Open

Dikluwe wants to merge 1 commit into
coin3d:masterfrom
Dikluwe:fix/sbplist-growth-exception-safety

Conversation

@Dikluwe

@Dikluwe Dikluwe commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve SbPList state when buffer growth allocation fails
  • reject capacity and expanding-subscript integer overflow before it becomes undefined behavior
  • add deterministic allocation-failure and INT_MAX regression coverage

Problem

SbPList::grow() currently updates itembuffersize before allocating the replacement buffer. If new[] throws, the old buffer remains installed while the list records the larger capacity. A later append then sees apparent spare capacity, skips growth, and writes past the actual buffer.

The same growth path can be reached through append, insert, expanding subscript, and copy/assignment into an existing smaller list.

Fix

Compute the target capacity locally, allocate and copy first, and publish the new buffer and capacity only after all potentially throwing work has succeeded. Since the stored elements are void *, copying them cannot throw, so allocation failure now provides the strong exception guarantee.

The change also guards doubling beyond INT_MAX / 2 and the INT_MAX + 1 calculation used by expanding subscript. The negative-size guard in expandlist() also protects callers compiled with the previous inline operator[] implementation.

There is no public API or object-layout change, and successful growth keeps the existing capacity policy.

Tests

  • isolated branch build and CTest: 9/9 passed
  • integration lab build and CTest: 18/18 passed
  • allocation regression deterministically fails the next array allocation and verifies length, capacity, backing-buffer identity, contents, and successful reuse
  • maximum-subscript regression verifies std::bad_alloc and unchanged list state

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant