[
https://issues.apache.org/jira/browse/FLINK-23147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17369374#comment-17369374
]
Arvid Heise edited comment on FLINK-23147 at 6/25/21, 10:15 AM:
----------------------------------------------------------------
> ehhhhhhh; a plugin interface with this signature CompletableFuture<Void>
> doSomething() could cause the same issue.
So far plugins didn't have life-cycle, so it wouldn't matter. Now that we have
life-cycle, we need probably need to expose it to the user somehow (e.g.
service can implement {{Closeable}}). After the plugins life-cycle ended, no
new classes can be loaded. That's fair imho.
> This statement is a bit confusing because I'm not sure whether you are
> describing the IS or the SHOULD state.
SHOULD state for both
> I wonder whether that would work in practice. I'd expect a fair amount of
> maintenance overhead to realize that. Say a plugin wants to se Preconditions;
> do we just say no? Bundle them in the plugin jar via brittle build setups?
> Whitelist Preconditions and all dependencies while having tests that ensure
> the API surface is clean?
We are marking stuff as internal to discourage users from using it. I don't see
why this is different for Preconditions. We can turn it into API and make it
accessible that way.
Or the users picks one of the many libraries that offer Preconditions (e.g.
Guava) and shade it into their plugins.
For me there is currently no difference in our Preconditions and
{{org.apache.flink.shaded.guava18.com.google.common.base.Preconditions}}. The
user shouldn't use any of that in plugins.
was (Author: arvid):
> ehhhhhhh; a plugin interface with this signature CompletableFuture<Void>
> doSomething() could cause the same issue.
So far plugins didn't have life-cycle, so it wouldn't matter. Now that we have
life-cycle, we need probably need to expose it to the user somehow (e.g.
service can implement {{Closeable}}).
> This statement is a bit confusing because I'm not sure whether you are
> describing the IS or the SHOULD state.
SHOULD state for both
> I wonder whether that would work in practice. I'd expect a fair amount of
> maintenance overhead to realize that. Say a plugin wants to se Preconditions;
> do we just say no? Bundle them in the plugin jar via brittle build setups?
> Whitelist Preconditions and all dependencies while having tests that ensure
> the API surface is clean?
We are marking stuff as internal to discourage users from using it. I don't see
why this is different for Preconditions. We can turn it into API and make it
accessible that way.
Or the users picks one of the many libraries that offer Preconditions (e.g.
Guava) and shade it into their plugins.
For me there is currently no difference in our Preconditions and
{{org.apache.flink.shaded.guava18.com.google.common.base.Preconditions}}. The
user shouldn't use any of that in plugins.
> ThreadPools can be poisoned by context class loaders
> ----------------------------------------------------
>
> Key: FLINK-23147
> URL: https://issues.apache.org/jira/browse/FLINK-23147
> Project: Flink
> Issue Type: Improvement
> Components: Runtime / Coordination
> Reporter: Chesnay Schepler
> Priority: Major
> Fix For: 1.14.0
>
>
> Newly created threads in a thread pool inherit the context class loader (CCL)
> of the currently running thread.
> For thread pools this is very problematic because the CCL is unlikely to be
> reset at any point; not only can this leak another CL by accident, it can
> also cause class loading issues, for example when using a {{ServiceLoader}}
> because it relies on the CCL.
> With the scala-free runtime this for example means that if an actor system
> threads schedules something into future thread pool of the JM then a new
> thread is created which uses a plugin loader as a CCL. The plugin loaders are
> quite restrictive and prevent the loading of 3rd-party dependencies; so if
> the JM schedules something into the future thread pool which requires one of
> these dependencies to be accessible then we're gambling as to whether this
> dependency can actually be loaded in the end.
> Because it is difficult to ensure that we set the CCL correctly on all
> transitions from akka to Flink land I suggest to add a safeguard to the
> ExecutorThreadFactory to enforce that newly created threads are always
> initialized with the CL that has loaded Flink.
> /cc [~arvid] [~sewen]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
