Skip to content

[Core] Fix the issues in BinaryPacket.GrowSize#910

Open
Executor-Cheng wants to merge 1 commit into
masterfrom
fix/GrowSize-bugs
Open

[Core] Fix the issues in BinaryPacket.GrowSize#910
Executor-Cheng wants to merge 1 commit into
masterfrom
fix/GrowSize-bugs

Conversation

@Executor-Cheng

@Executor-Cheng Executor-Cheng commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Fix the issue where BinaryPacket.GrowSize enters an infinite loop and fails to return the shared buffer.

使用无参构造BinaryPacket时调用BinaryPacket.GrowSize函数会导致陷入死循环。
不清楚开发者意图是什么,或许应该禁用无参构造。

@Controllerdestiny 没有方法能够禁止调用结构体的默认构造, 总是能通过 default(T) 创建数据全零的结构体
顺带修复扩容时有借无还的问题

扩容算法摘自 Hacker’s Delight 2nd edition, Chapter 3. Power-of-2 Boundaries, 3–2 Rounding Up/Down to the Next Power of 2
原来BCL给了 BitOperations.RoundUpToPowerOf2

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates BinaryPacket.GrowSize to avoid an infinite loop when a BinaryPacket is default-initialized (capacity = 0), and to correct buffer pooling behavior during growth.

Changes:

  • Replaces the _capacity *= 2 growth loop with a computed target capacity based on rounding.
  • Adjusts buffer rent/return behavior when resizing to use ArrayPool<byte>.Shared.
  • Updates internal state (_capacity, _bytesToReturnToPool, _span, _buffer) during growth.

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

Comment thread Lagrange.Core/Utility/Binary/BinaryPacket.cs Outdated
Comment thread Lagrange.Core/Utility/Binary/BinaryPacket.cs
Comment on lines 293 to +297
private void GrowSize(int additional)
{
while (_offset + additional > _capacity) _capacity *= 2;
_bytesToReturnToPool = ArrayPool<byte>.Shared.Rent(_capacity);
int requested = _offset + additional;
int desired = 1 << -BitOperations.LeadingZeroCount((uint)requested);
int capacity = desired > requested ? desired : requested;
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.

2 participants