Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007253OCamlstandard librarypublic2016-05-10 11:082018-05-21 11:38
Reporterdbuenzli 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusacknowledgedResolutionopen 
PlatformOSOS Version
Product Version4.03.0 
Target VersionlaterFixed in Version 
Summary0007253: at_exit functions get called twice if a callback raises and prevents earlier handlers to execute.
DescriptionThe behaviour in case of an [at_exit] callback raising is a little bit suboptimal (slightly related to 0007178):

1. A raising callback prevents callbacks registered earlier to be executed.
2. A raising callback and all the callbacks that come after get called twice.
Steps To Reproduce(* 1. prevents callbacks from running *)

# let () = at_exit (fun () -> print_endline "Once";);;
# let () = at_exit (fun () -> print_endline "Twice"; raise Exit);;
# ^D
Twice
Twice
Fatal error: exception Pervasives.Exit
...

(* 2. Double calls *)

# let () = at_exit (fun () -> print_endline "Twice"; raise Exit);;
# let () = at_exit (fun () -> print_endline "Once";);;
# ^D
Once
Twice
Once
Twice
Tagsjunior_job
Attached Files

- Relationships
related to 0007796confirmed "at_exit" function run twice with cleanup-at-exit runtime option 

-  Notes
(0015910)
dbuenzli (reporter)
2016-05-10 11:19

Forgot to add, this is not a toplevel problem. The problem also occurs on executables.
(0015911)
dbuenzli (reporter)
2016-05-10 11:30
edited on: 2016-05-10 11:30

The fix doesn't seem too hard: the call to the callbacks in the chaining should be protected by handler here:

https://github.com/ocaml/ocaml/blob/393f068a1d4e0b267012b10aac822d11160a7cfd/stdlib/pervasives.ml#L515 [^]

The double call is likely to come from the call to Pervasives.do_at_exit by printexc.ml on unhandled exception:

https://github.com/ocaml/ocaml/blob/393f068a1d4e0b267012b10aac822d11160a7cfd/stdlib/printexc.ml#L260 [^]

So if at_exit chaining properly catches exceptions and prints them I guess the double call should disappear.

(0015956)
dexterph (reporter)
2016-06-01 11:12

The double calls do disappear, however there is no clear way to print an exception from within at_exit in Pervasives. I think a more complicated change would need to happen.
(0016051)
dbuenzli (reporter)
2016-07-11 13:19

So I have a fix for this which works so that each uncaught exception in at_exit functions get reported. However this slightly changes the semantics of the callback set by Printexc.set_uncaught_exception_handler function (introduced in 4.02) since it is supposed there that:

"Note that when fn is called all the functions registered with at_exit have already been called. Because of this you must make sure any output channel fn writes on is flushed."

With this fix, the "have already been called" becomes "may already have been called", since I try to call the at_exit handlers as much as possible until one raises in which case I record the raise, handle the uncaught exception and then handle the at_exit function uncaught exception using the same mechanism.

This could be one option. However I also have the feeling that this way of operating makes it less easy to understand what is happening, for example:

# let () = at_exit (fun () -> print_endline "bla");;
# let () = at_exit (fun () -> raise Exit);;
# let () = at_exit (fun () -> print_endline "bli");;
# let () = at_exit (fun () -> failwith "HEY");;
# ^D
bli
Fatal error: exception Failure("HEY")
Raised at file "pervasives.ml", line 32, characters 22-33
Called from file "pervasives.ml", line 519, characters 35-39
Called from file "pervasives.ml", line 527, characters 4-17
Called from file "toplevel/toploop.ml", line 547, characters 21-27
Called from file "toplevel/topstart.ml", line 16, characters 8-22
bla
Fatal error: exception Pervasives.Exit
Raised at file "//toplevel//", line 1, characters 34-38
Called from file "pervasives.ml", line 519, characters 35-39
Called from file "printexc.ml", line 263, characters 12-36


So here bli gets printed first because HEY gets uncaught, we try to call as much as possible the at_exit handlers, which leads bli to get called, then Exit is uncaught, so we print HEY and start the uncaught loop for Exit, which leads bla to get called and finally Exit to be printed.

Now it would be as easy and possible to execute and print this in the natural order. So my question is what was the rationale behind calling at_exit functions before the handler ? Would it be a problem to change the semantics so that:

"Note that when fn is called all functions that are registered with at_exit that have not been called yet will be called after the call to the handler" ?
(0016058)
dbuenzli (reporter)
2016-07-12 19:11

A fix is proposed in https://github.com/ocaml/ocaml/pull/685. [^] W.r.t. to my previous message it is the version that tries to retain a natural call order w.r.t. to effects.
(0016836)
shinwell (developer)
2016-12-08 10:51

@dbuenzli: Has there been any further progress on this? GPR#685 appears to have been closed without being merged.
(0016857)
dbuenzli (reporter)
2016-12-08 12:32

No. I closed it because it was not a good solution.
(0016859)
shinwell (developer)
2016-12-08 12:38

Do you want to keep this Mantis issue open or not?
(0016860)
dbuenzli (reporter)
2016-12-08 12:40

Well it is an issue no ?
(0019125)
xleroy (administrator)
2018-05-21 11:38

Alternate fix proposed here: https://github.com/ocaml/ocaml/pull/1790 [^]

- Issue History
Date Modified Username Field Change
2016-05-10 11:08 dbuenzli New Issue
2016-05-10 11:19 dbuenzli Note Added: 0015910
2016-05-10 11:30 dbuenzli Note Added: 0015911
2016-05-10 11:30 dbuenzli Note Edited: 0015911 View Revisions
2016-05-10 15:34 doligez Status new => acknowledged
2016-05-10 15:34 doligez Target Version => 4.04.0 +dev / +beta1 / +beta2
2016-05-10 15:34 doligez Tag Attached: junior_job
2016-06-01 11:12 dexterph Note Added: 0015956
2016-07-11 13:19 dbuenzli Note Added: 0016051
2016-07-12 19:11 dbuenzli Note Added: 0016058
2016-09-08 11:17 shinwell Target Version 4.04.0 +dev / +beta1 / +beta2 => later
2016-12-08 10:51 shinwell Note Added: 0016836
2016-12-08 10:51 shinwell Category OCaml runtime system => OCaml standard library
2016-12-08 12:32 dbuenzli Note Added: 0016857
2016-12-08 12:38 shinwell Note Added: 0016859
2016-12-08 12:40 dbuenzli Note Added: 0016860
2017-02-23 16:43 doligez Category OCaml standard library => standard library
2018-05-17 13:25 xleroy Relationship added related to 0007796
2018-05-21 11:38 xleroy Note Added: 0019125


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker