[
https://issues.apache.org/jira/browse/FLINK-15156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17215561#comment-17215561
]
Hwanju Kim commented on FLINK-15156:
------------------------------------
Robert, thanks for the comments:
# Performance penalties: Good question. Although we haven't microbenchmarked
its impact, we haven't heard performance regression. Actually, all of the
security check is not on data path more in control path (thread control, socket
accept/connect, exit, etc), I did not think it would matter. More importantly,
if there is no added check, it's just a single function call with null
check/return (and JDK also by default does one getSecurityManager call and null
check as well). So considering that additional path is very small in control
path, I think overhead should be negligible. Nonetheless, for platform
provider, such security manager for sandboxing is inevitable going forward not
only for exit, which is just the first usecase, even if it may cause a bit of
penalties. That's ene reason to have configuration that defaults to false for
general users (not platform providers).
# More check points: Right. Thanks for the input. I also tried (ambitiously)
to find all the places in the first place, but it was not very practical (also
code addition may make patch larger) at that point so decided to add the
frequently implemented path. Indeed, main() is the one where exit() is (most
likely erroneously) put blindly following the same practice for general Java
application. I haven't had data for the case where users put exit() in UDF
though (but it may be possible if some badly written library can call it behind
the scene, but I haven't seen that yet). We can further discuss about what more
points are interesting.
# In the case I have seen, mostly their exit is with log already (e.g., this
argument is not provided, exiting) and we wanted to put the guard not
terminating JVM by further messaging "you are not allowed to terminate JVM". I
thought throwing exception with message is providing user with
warning/debugging message without terminating JVM. Do you think some users
still want their exit to be performed intentionally? I assumed that UDF or main
terminating JVM is always unintended in Flink. But if exit may still be a
wanted behavior in some cases, I think it makes sense to provide that option as
well.
> Warn user if System.exit() is called in user code
> -------------------------------------------------
>
> Key: FLINK-15156
> URL: https://issues.apache.org/jira/browse/FLINK-15156
> Project: Flink
> Issue Type: Improvement
> Components: Runtime / Coordination
> Reporter: Robert Metzger
> Priority: Minor
> Labels: starter
>
> It would make debugging Flink errors easier if we would intercept and log
> calls to System.exit() through the SecurityManager.
> A user recently had an error where the JobManager was shutting down because
> of a System.exit() in the user code:
> https://lists.apache.org/thread.html/b28dabcf3068d489f38399c456c80d48569fcdf74b15f8bb95d532d0%40%3Cuser.flink.apache.org%3E
> If I remember correctly, we had such issues before.
> I put this ticket into the "Runtime / Coordination" component, as it is
> mostly about improving the usability / debuggability in that area.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
