Skip to content

Remove invalid asserts from ASIO system#4618

Merged
SeanTAllen merged 1 commit intomainfrom
asio-asserts
Feb 18, 2025
Merged

Remove invalid asserts from ASIO system#4618
SeanTAllen merged 1 commit intomainfrom
asio-asserts

Conversation

@SeanTAllen
Copy link
Copy Markdown
Member

There's an ABA condition that can exist with how the ASIO system works in the runtime. There's guard
conditions in various functions to ditch early for scenarios where this is encountered.

A few years ago, some asserts were added to those guard clauses. Unfortunately, that defeats the purpose of the guards and will eventually result in assertions being triggered under load with a debug runtime.

Without completely rewriting how the ASIO system works to be ABA friendly, these asserts are a bad idea. I don't feel up to doing a rewrite at this time, so I am removing the asserts.

There's an ABA condition that can exist with how the ASIO system
works in the runtime. There's guard
conditions in various functions to ditch early for scenarios where
this is encountered.

A few years ago, some asserts were added to those guard clauses.
Unfortunately, that defeats the purpose of the guards and will
eventually result in assertions being triggered under load with a
debug runtime.

Without completely rewriting how the ASIO system works to be ABA
friendly, these asserts are a bad idea. I don't feel up to doing a
rewrite at this time, so I am removing the asserts.
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 17, 2025
@SeanTAllen SeanTAllen merged commit 9c95489 into main Feb 18, 2025
28 checks passed
@SeanTAllen SeanTAllen deleted the asio-asserts branch February 18, 2025 00:48
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Feb 18, 2025
SeanTAllen added a commit that referenced this pull request Mar 7, 2025
SeanTAllen added a commit that referenced this pull request Mar 7, 2025
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