Skip to content

Conversation

@doug-q
Copy link
Contributor

@doug-q doug-q commented Jul 9, 2025

No description provided.

@ss2165 ss2165 removed their request for review September 24, 2025 09:38
@jake-arkinstall
Copy link
Collaborator

Rebased on main. I'll make a PR to this branch with additions for the simulator and error model plugins too

@jake-arkinstall
Copy link
Collaborator

jake-arkinstall commented Nov 7, 2025

An issue with this - and the reason that I am not comfortable merging just yet - is program flow.

The hard part is in how teardown is managed following a panic. I've noticed that, in a certain internal runtime plugin, its own panic handling is allowing it to teardown gracefully later on (I have not investigated the mechanism at this point). When catching the unwind at a higher level instead, it is not so graceful - our assumption that a panicked component is still capable of tearing down is leading to complaints such as free(): invalid size when shot_end is called.

What I'd like to suggest is that we hold off on this for now, and require plugins to manage their own panic behaviour (if any) and to provide a panic-free interface for selene to use. If there are lifetime concerns, they can manage it with their own internal state (e.g. maintaining an "invalid_state" flag).

Then, in the next breaking release, we establish a mechanism for handing different forms of error. I would particularly like to add something that allows us to assume a panicked component is not to have a teardown attempted. This is currently difficult because we do not have formal error code regions aside from:

  • <1000: exit shot, start next
  • >=1000: user program panic, graceful teardown

as such, a component panic is shoehorned into the latter.

Adding a:

  • >=100000: internal component panic, no graceful teardown

would be compatible with the error codes used internally, yet I believe it would be a breaking change for the purpose of requiring user inputs be bounded, and we would additionally need to do a lot of testing to ensure that the 'no graceful teardown' is still sufficiently graceful for the result stream handler to remain graceful.

This is also in line with your suggestion about giving error codes names.

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.

3 participants