Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Exception is never raised in fm_dispatch due to self-kill #9989

Open
berland opened this issue Feb 6, 2025 · 3 comments · May be fixed by #10102
Open

Exception is never raised in fm_dispatch due to self-kill #9989

berland opened this issue Feb 6, 2025 · 3 comments · May be fixed by #10102
Assignees
@jonathan-eq
Labels
bug

Comments

@berland
Copy link
Contributor

berland commented Feb 6, 2025

The os.killpg kills the fm_dispatch process such that the exception e is never raised. If there is a bug in fm_dispatch that triggers an exception, it will not be printed to the terminal. Running fm_dispatch in the runpath only gives you the output Terminated.

ert/src/_ert/forward_model_runner/fm_dispatch.py

Lines 194 to 199 in 68271ad

try:
fm_dispatch(sys.argv)
except Exception as e:
pgid = os.getpgid(os.getpid())
os.killpg(pgid, signal.SIGTERM)
raise e

@berland berland moved this to Todo in SCOUT Feb 6, 2025
@berland berland added the bug label Feb 6, 2025
@jonathan-eq jonathan-eq self-assigned this Feb 13, 2025
@jonathan-eq jonathan-eq moved this from Todo to In Progress in SCOUT Feb 13, 2025
@jonathan-eq
Copy link
Contributor

I can change it to another signal, and add a signal_handler connected to proc.kill() in forward_model_step.py. How about signal.SIGQUIT? @berland

@jonathan-eq jonathan-eq linked a pull request Feb 18, 2025 that will close this issue
Open
9 tasks
@berland
Copy link
Contributor Author

berland commented Feb 18, 2025

It is dangerous to remove the killpg() call as it ensures every descendant is taken down. What about just printing e and then kill? Add an random exception somewhere in fm_dispatch and check the developer experience.

@jonathan-eq
Copy link
Contributor

jonathan-eq commented Feb 20, 2025 •
edited
Loading

But what about the exit code if we only print the exception? Will it default to 0, or will it be set due by SIGKILL?
EDIT: The exit code is not 0 when we SIGKILL, so it should be fine :)

@jonathan-eq jonathan-eq moved this from In Progress to Ready for Review in SCOUT Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonathan-eq jonathan-eq

Labels
bug
Projects
SCOUT
Status: Ready for Review
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix muted exceptions in fm_dispatch due to self SIGKILL jonathan-eq/ert
2 participants
@berland @jonathan-eq