-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Every time ensure
clause runs even when program terminated with exit
#1935
Comments
You are right. But what's wrong with |
First there is a consistency issue. When writing a program I expect every ensure block to run. They may include database shutdown, transaction commits or aborts, graceful or fast network disconnections, logging or other code that must run before exiting. Consider an application running multiple http services + opening multiple key value databases. Any service failure should stop all other services. The databases should be gracefully closed last. In the following example of any of the services call
Should every key value database register an |
Most programs don't call |
Thanks @technorama. Your description is detailed and I think so too. |
I really can't imagine a long-running application that in between its lines calls exit. For example, doing an Java works that way too, and I never heard anyone complaining about it. Go also works that way: "The program terminates immediately; deferred functions are not run.". And people are writing wonderful, robust services in Go. I really don't think we should complicate the language and the exception hierarchy because of |
When you say exception hierarchy you mean a base exception class and a SystemExit/ExitException/Exception::Exit class? Two additional classes aren't really that complicated and both ruby and python have analogues. An alternative I guess is providing a way to indicate exit status when raising an exception. |
Have any decisions been made on this in some other discussion? I have also been wondering about interrupts. In ruby an interrupt is processed as an exception and you can optionally rescue it or ensure cleanup code execution. With crystal your only recourse it to trap the interrupt explicitly and figure out what needs to be cleaned up inside the trap. This behaviour also lets ruby's at_exit callbacks fire if a program was interrupted without having to trap anything. |
@technorama If you need to exit with a specific status code and ensure to run some tasks, you can use an application-specific termination method which calls cleanup handlers and before calling But these are two ways to accomplish the desired behaviour with minimal effort. I don't think it makes sense to add additional overhead to the language for a feature with pretty rare use case. |
@straight-shoota What overhead could be relevant for an abnormal code path? Where are the benchmarks to prove running ensure and exception handlers add any significant amount of runtime overhead especially considering the normal program path has the exact same overhead? Code overhead is 2 empty classes, a rescue and a small change to exit(). What overhead are you speaking of? The current behavior violates the principle of least surprise. The behavior of ruby/python is what most programmers both want and expect especially when designing libraries. Some examples where this falls apart (using partial/simplified code):
In the example the following surprises happen:
By the time these programs make it to the ops team they have no idea about how to handle the subtle failures/inconsistencies. They just tend to hate the program for failing in unexpected ways and advocate it's disuse. Ask me sometime about a program that used to take over 16 hours to rebuild it's indices if they were out of sync from the main db file. Libraries:
You could remove exit() and only provide an exit!() with warnings to never call it unless you know what you doing, have written or read every library used in the program yourself and understand all the implications of skipping ensure/rescue blocks within the particular program. Not providing a standardized set of exceptions and reliable ensure clauses is setting the stage for subtle bugs that every programmer will handle differently. The following should demonstrate why an exception hierarchy is necessary for writing robust code (ruby/python figured this out ages ago).
There's still the issue of attempting to exit with a non 0 or 1 value and providing a safe way to do so where all library/program authors expectations are met. raise SystemExit(exit_code: 255) could be used as an alternative but this has other problems without a standardized exception hierarchy. Code overhead is 1-2 empty classes, a rescue and a small change to exit(). Really, what's the big deal? |
Well, another use case I stumbled upon is in specs where you need to do some cleanup like deleting a tempfile and you want it to clean up even if the spec itself is aborted (Strg+C). But I don't think the proposed solution could actually make this work as |
Being able to "catch" an exit massivle violates the priciple of least surprise to me. People coming from ruby and python will expect an exit exception, people coming from any other language (java, C#, C++, C, Go, Rust, I could go on) this is absolutely not what they expect. Because having an "exit exception" is asking for the impossible. It's not reasonable to expect to be able to clean up in every situation. Raise can fail, you can segfault, you can get |
I strongly vote close. |
There's always going to be cases where the program terminates and cannot clean up, that doesn't mean we shouldn't create mechanisms for clean up. Additionally those Java and C# programmers might not expect an exit exception, but they won't see one either, since the ExitException or SignalException would have to be explicitly rescued by type or with rescue Exception, which is expected to be bad practice in every language. As for the threading problem with signals, it is possible to raise from the real signal handler, not the spawned one listening on the loopback pipe. This behaviour can be used to send a signal interrupt exception through to the currently executing Fiber, and with a bit more work you can send that same exception to every currently executing Fiber when they are resumed. This actually allows a single fibered app or long running fibers to be properly interrupted by an incoming signal instead of having to explicitly yield, in the case that you have registered a signal handler. The idea would be to create default signal handlers for signals that have default exit behaviour (SIGQUIT, SIGINT). This default handler would send the signal into the loopback pipe and then raise a SignalException for that signal. The currently executing fiber then gets to respond to that signal and if the exception is not rescued the fiber terminates. Eventually the libevent loop fiber is reached and reads the signal on the loopback pipe. It looks at the currently running fibers and schedules an interrupt for each of them. When that fiber is resumed it looks at it's interrupt queue and raises any interrupt from there. This way all fibers get to respond to the interrupt. In order to not affect current developers relying on trap functionality, trap would have an extra parameter 'raises' that would determine if the real handler raised an exception or not and by default it would not. This would mean that users who wrote code with no traps would get similar behaviour (their program would exit due to exception instead of kernel termination), while users who used traps would get the same behaviour (their program would ignore the interrupt and the kernel would run their handler instead) unless they changed it to also raise by their choice. There are issues with receiving multiple signals, but it's still better than hard exiting. You can write your program to be robust on start up and shutdown. |
@strigonLeader Exit exceptions are just incredibly leaky abstractions which quite rightly haven't been picked up by any other language. |
Exit means that the program must stop, otherwise you'd raise an exception, so ensure blocks are called, and the exception be handled (even if just to exit cleanly). We can run cleanup code using We could tell the scheduler to raise an exception when a fiber tries to stop/resume, but what if there are a hundred thousand fibers? Shall we raise a hundred thousand exceptions every time the program exits? I don't think the improvement is worth the complexity. I'm not even sure that would be an improvement. I'm also in favour of closing. I don't think we'll implement this. |
In Python and Ruby, see following programs:
while program terminated with
exit
function/method, both programs display "Exit!". It means we expectensure
clause (also calledfinally
clause) runs every time.On top of that, it's interested, both Python and Ruby implement
exit
by throwingSystemExit
exception.SystemExit
is one of special exception, which is not caught bycatch
clause usually. So if program catchesSystemExit
exception, program is not terminated. To terminate program immediately, Ruby hasexit!
method, and Python hasos._exit
too. These method/functions are dangerous.I suggested spec that is such a following:
SystemExit
classexit
method raises aSystemExit
, butcatch
clause does not catch this object normally.SystemExit
object and decides a return code.and discussion points are:
SystemExit
class have? Should doesSystemExit
inheritException
?exit!
like method to Crystal? How is difference betweenexit!
andLibC.exit
? Or, isexit!
alias toLibC.exit
?catch
clause have the ability to catchSystemExit
? But we finally catchSystemExit
in main function.Fiber
has same problem to runensure
clause. If program was terminated, should we runensure
clause by sendingSystemExit
to all fibers?ensure
clause run every time? For example in Java, it cannot runsfinally
clause when program terminated bySystem.exit
. (I believe not, but it exists for fair discussion.)(This issue is connected with #1921)
References:
Python's
sys.exit
: https://docs.python.org/3/library/sys.html#sys.exitRuby's
exit
: http://ruby-doc.org/core-2.1.1/Kernel.html#method-i-exitJava's
System.exit
: https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#exit-int-The text was updated successfully, but these errors were encountered: