[ 
https://issues.apache.org/jira/browse/FLINK-21307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17279761#comment-17279761
 ] 

Piotr Nowojski commented on FLINK-21307:
----------------------------------------

I have two concerns here.

1. I think currently there are a couple of bugs. For example at the end of the 
{{StreamTask#triggerCheckpoint}}, {{FlinkSecurityManager}} is being disabled, 
while this method is invoked from the task/mailbox thread. So it would disable 
{{FlinkSecurityManager}} for any subsequent mailbox actions and record 
processing which is happening in the task/mailbox thread (virtually all user 
code invocation are happening there). At the very least, we should never turn 
off/turn on {{FlinkSecurityManager}}, but we should be restoring pre-existing 
setting (if it has already been enabled before calling 
{{StreamTask#triggerCheckpoint}}, we should leave it enabled when leaving 
{{StreamTask#triggerCheckpoint}}.

2. Is {{FlinkSecurityManager}} enabled in {{LegacySourceFunctionThread}}?

3. This also shows a bit fragile contract, where we are enabling/disabling 
{{FlinkSecurityManager}} in random places. I would propose to change this to 
either:
  a) enable it only just before touching the user code (might be complicated 
and cause some overhead in the critical per-record code paths)
  b) enable it always whenever we are creating a new thread, that might be 
executing user's code (task canceller thread, task thread, 
LegacySourceFunctionThread, ...?), and manually/explicitly disable/bypass it 
whenever we want Flink to exit via {{System.exit()}}. For example FLINK-21306. 
In other words we would assume that all {{System.exit()}} calls are coming from 
the user code, unless they are issued from {{FlinkSecurityManager}}. 

Is there a way maybe to enable {{FlinkSecurityManager}} by default for every 
thread? If so, that would help us to handle custom threads spawned by operators 
(sources, sinks, {{AsyncWaitOperator}}, ...)/3rd party libraries (S3, ...) etc.

> Revisit activation model of FlinkSecurityManager
> ------------------------------------------------
>
>                 Key: FLINK-21307
>                 URL: https://issues.apache.org/jira/browse/FLINK-21307
>             Project: Flink
>          Issue Type: Improvement
>          Components: Runtime / Task
>    Affects Versions: 1.13.0
>            Reporter: Robert Metzger
>            Priority: Major
>
> In FLINK-15156, we introduced a feature that allows users to log or 
> completely disable calls to System.exit(). This feature is enabled for 
> certain threads / code sections intended to execute user-code.
> The activation of the security manager (for monitoring user calls to 
> System.exit() is currently not well-defined, and only implemented on a 
> best-effort basis.
> This ticket is to revisit the activation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to