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

Terminate the process in case of an unhandled exception #2804

Closed
wants to merge 3 commits into from

Conversation

mvicsokolova
Copy link
Contributor

Fixes #2398

elizarov and others added 2 commits May 14, 2021 18:23
* Provides newSingleThreadedContext.
* Provides Dispatchers.Main on iOS, Dispatchers.Default everywhere.
* Coroutine references (Job), all kinds of channels and StateFlow are shareable across workers.
* Each individual coroutine is confined to a single worker.
* Update Dispatchers docs to account for native-mt changes.
* Multithreaded support in select expression.
* Fix ObjC autorelease object leaks with native-mt dispatchers (#2477)

Additional fixes:
* Fixed broadcast builder with different thread
* Fixed adding a child to a frozen parent job

Fixes #462
Fixes #470
Fixes #765
Fixes #1645
Fixes #1751
Fixes #1828
Fixes #1831
Fixes #1764
Fixes #2064
Fixes #2025
Fixes #2226
Fixes #2138
Fixes #2263
Fixes #2322
Fixes #2283
Fixes #2688
@mvicsokolova mvicsokolova requested a review from qwwdfsad July 2, 2021 13:27
throw exception
// Use unhandled exception hook if it is set, otherwise print the stacktrace
val hook = setUnhandledExceptionHook({ _: Throwable -> }.freeze())
if (hook != null) hook(exception) else exception.printStackTrace()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worth prepending the coroutine context and the fact that an app is terminated due to unhandled exceptions before calling exitProcess(-1)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm.... I see that this PR is related to native and I am not concerned at the moment. Also I don't do mobile an don't know the details of setup, so my question might look stupid. But can we be sure that this change is confined to iOS? In other words, are we sure that kotlin native with coroutines will never be used for server side or desktop development? Because exitProcess(-1) seems too radical for me on the server, I would definitely hate it. Again sorry for stupid question.

Copy link
Member

@qwwdfsad qwwdfsad Jul 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your question raised an important issue that we're going to address, thanks for that!

It is indeed important to have an ability not to terminate an application on unhandled exceptions, but it also important to be able to do so.
KMM and iOS is our primary focus for now and there is only an API to do the latter, not the former.

So, in the 1.5.1-native-mt the default behaviour will be to crash (in the end, an unhandled exception in a coroutine is no different from the unhandled exception from main or a worker) for the sake of iOS apps. The workaround to avoid application crashes is not to leave exceptions unattended (e.g. by using CoroutinesExceptionHandler).

But in 1.5.30 (or in 1.6.0, it's being discussed right now) we'll provide a new K/N stdlib API that allows to invoke unhandled exception hook without termination (right now this API is supposed to be used only with the consecutive call to abort or exitProcess) and will provide an opt-in mechanism to set your own exception hook without crashing.

@qwwdfsad qwwdfsad force-pushed the native-mt branch 4 times, most recently from 8df9c27 to 48cf2cf Compare July 12, 2021 13:32
@qwwdfsad
Copy link
Member

Merged manually

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.

4 participants