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.

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

Provide Thread.set_uncaught_exception_handler #6813

Closed
vicuna opened this issue Mar 13, 2015 · 17 comments
Closed

Provide Thread.set_uncaught_exception_handler #6813

vicuna opened this issue Mar 13, 2015 · 17 comments

Comments

@vicuna
Copy link

vicuna commented Mar 13, 2015

Original bug ID: 6813
Reporter: @whitequark
Assigned to: @diml
Status: assigned (set by @diml on 2015-03-14T10:21:20Z)
Resolution: open
Priority: normal
Severity: feature
Target version: 4.07.0+dev/beta2/rc1/rc2
Category: otherlibs
Monitored by: @hcarty

Bug description

Original summary was: Uncaught exceptions inside threads don't trigger handler set via Printexc.set_uncaught_exception_handler

See summary.

@vicuna
Copy link
Author

vicuna commented Mar 18, 2015

Comment author: @diml

I'm not sure uncaught exceptions inside threads should call the handler set via [Printexc.set_uncaught_exception_handler]: the doc says it is executed after [at_exit] handlers and doesn't require the callback to be thread-safe.

On the other hand it's easy to add a [Thread.set_uncaught_exception_handler]. For multi-threaded programs the handler should probably print the thread id so it make sense to have two different functions.

@vicuna
Copy link
Author

vicuna commented Mar 18, 2015

Comment author: @whitequark

That would work, indeed.

@vicuna
Copy link
Author

vicuna commented Jan 5, 2017

Comment author: @diml

I had another look at this, it should be simpler than the global uncaught exception handler given that the exception is caught on the OCaml side. Do you think you could submit a PR for this? I can review it

@vicuna
Copy link
Author

vicuna commented Jan 6, 2017

Comment author: @whitequark

I don't have time for this.

@vicuna vicuna added this to the 4.07.0 milestone Mar 14, 2019
@vicuna vicuna assigned ghost Mar 14, 2019
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 11, 2020
@whitequark
Copy link
Member

Still an issue.

@github-actions github-actions bot removed the Stale label Jun 15, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jun 16, 2021
@whitequark
Copy link
Member

Still an issue.

@gasche
Copy link
Member

gasche commented Jun 16, 2021

If we wait again for a few years to fix this issue, it will probably happen on the Multicore runtime. I wonder how uncaught exceptions are handled there; @kayceesrk, @Engil?

@gasche gasche removed the Stale label Jun 16, 2021
@abbysmal
Copy link
Contributor

@gasche On Multicore, uncaught exceptions in (sys)threads are handled in the same way.
There is a default handler around every Thread.t created through Thread.create, calling back to caml_thread_uncaught_exception to print the backtrace and thread id.

I think the proposal to add Thread.set_uncaught_exn works and it may be easy enough to implement:

  • In Multicore, how do we handle this in a multi-domains, multi-systhreads setting? I think this could survive as a global, assuming users are making sure the default handler is set only once, by one domain. We can just CAS it away as well.
  • In both trunk and Multicore: this handler should not raise an exception. I think we can have the suggested Thread.set_uncaught_exception_handler install a default handler as well on top of it. (in the same way it is done in Thread.new, except for the custom handler itself.)

As far as Multicore is concerned, this is different for the Domain API: uncaught exception within a Domain are only propagated on join.
The custom default exception handler is only called when such exception are propagated to the initial domain if I'm not mistaking. (@kayceesrk may confirm this)

@gasche
Copy link
Member

gasche commented Jun 16, 2021

Thanks for the answer. Here is what I understand:

  • domains propagate uncaught exceptions to their "joiner", so there is no need for a domain-specific mechanism
  • for threads the question is the same on Multicore and non-Multicore, we want a runtime global (not domain-local) that stores the current thread-exception-handler.

Would you by chance be interested in submitting a PR to implement this?

@abbysmal
Copy link
Contributor

Sure enough, I will give it a try in the next few days.

@github-actions
Copy link

github-actions bot commented Jul 4, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jul 4, 2022
@Octachron
Copy link
Member

There is still some planned work in OCaml 5 (after 5.0) to better handle uncaught exceptions on both threads and domains.

@dbuenzli
Copy link
Contributor

dbuenzli commented Jul 4, 2022

There's an issue and discussion here #11074

@github-actions github-actions bot removed the Stale label Jul 6, 2022
@dbuenzli
Copy link
Contributor

Albeit deemed unsatisfactory, formally OCaml 5 provides that. I think this can be closed. #11074 has a larger discussion on trying to improve things.

@gasche gasche closed this as completed Dec 23, 2022
@gasche
Copy link
Member

gasche commented Dec 23, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants