Skip to content

PR#7209: do not run at_exit handlers on exec failure#532

Merged
damiendoligez merged 1 commit into4.03from
unknown repository
Apr 4, 2016
Merged

PR#7209: do not run at_exit handlers on exec failure#532
damiendoligez merged 1 commit into4.03from
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 4, 2016

Fix for PR#7209: use sys_exit instead of exit in the child process when execv* fails.

The various functions were inconsistent in the way they handled exceptions raised by code between the fork () = 0 and the exec call. Some such as create_process handled them the same as exec exceptions and exited with 127. Other such as open_proc did not handle them, and I suppose let them leak to the caller which doesn't seem right. I changed the latter ones to behave the same as the former ones: i.e. any exception in the child process leads to sys_exit 127.

It was less clear what to do for establish_server so I didn't change it

@damiendoligez
Copy link
Copy Markdown
Member

It looks to me that establish_server should call sys_exit 0 rather than exit 0. Can you explain why it's not clear to you?

@damiendoligez damiendoligez added this to the 4.03.0 milestone Apr 4, 2016
@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 4, 2016

Because of the call to server_fun. This function is provided by the user and might do arbitrary things, including registering at_exit handlers and expecting them to be executed

@damiendoligez
Copy link
Copy Markdown
Member

Ah ok. What we would like to do is clear the at_exit list right after the fork, in fact.

@damiendoligez damiendoligez merged commit 9776f30 into ocaml:4.03 Apr 4, 2016
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request May 17, 2021
…er_test

Addition of test for finaliser callback with major cycle
stedolan pushed a commit to stedolan/ocaml that referenced this pull request May 24, 2022
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.

1 participant