Skip to content

Conversation

@JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Jan 12, 2026

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fixes #1887
Fixes #2342

Marked UnmanagedMemoryHandle properties and methods as readonly for improved immutability and performance. Refactored UniformUnmanagedPoolMemoryAllocatorTests to remove platform/CI checks, use culture-invariant parsing, and enhance finalization tests to assert handle pooling via handle count invariants rather than memory content or pointer values.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the UnmanagedMemoryHandle struct and refactors finalization tests to be more robust and reliable across platforms. The changes address issues #1887 and #2342 related to flaky finalization tests on macOS and ARM64.

Changes:

  • Added readonly modifiers to UnmanagedMemoryHandle properties and methods for improved struct immutability
  • Removed platform-specific and CI-only test skips that were masking finalization test issues
  • Refactored finalization tests to use handle count invariants instead of unreliable sentinel value checks

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs Added readonly modifiers to properties (Handle, IsInvalid, IsValid, Pointer) and methods (Equals, GetHashCode) for better struct semantics
tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs Removed platform/CI checks, added culture-invariant parsing, and refactored finalization tests to verify pooling via TotalOutstandingHandles count instead of memory content comparison

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JimBobSquarePants JimBobSquarePants merged commit 498a0a0 into main Jan 12, 2026
11 checks passed
@JimBobSquarePants JimBobSquarePants deleted the js/memoryallocator-validation branch January 12, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AllocateMemoryGroup_Finalization_ReturnsToPool test fails on Linux ARM64 AllocateSingleMemoryOwner_Finalization_ReturnsToPool test fails on mac os

2 participants