Skip to content

Conversation

@matheusd
Copy link
Contributor

Cherry-picked from #586.

That PR has uncovered more races, so I'm trying again with a different approach to eliminate them.

The commits that were unrelated to the actual refactor are in this PR to ease the future efforts.

The arena was reused when it shouldn't after a Release() call.
In the future, it would be ideal if wrongful Release() calls could be
flagged (either through an error or panic).
This moves the check for released messages earlier in the Release() call
for transport messages. This prevents triggering the race detector, due
to the check happening before an attempt is made at accessing the
message (which involves indirection through a segment that may have been
released already).

While the prior code would not have actually caused a fault in current
code (because the conditional checks both for the released flag and
whether the message is nil), checking by the release flag first is
more correct and may prevent future bugs.
Previously this was not enforced by the Encode function. While this
limit is unlikely to be hit in practice, it's important to assert it due
to the type allowing it and a count of segments greater than 2^32
causing an invalid marshaling.
@lthibault lthibault merged commit 396906c into capnproto:main Aug 20, 2024
@matheusd matheusd deleted the some-fixes branch August 21, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants