[
https://issues.apache.org/jira/browse/MRESOLVER-278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17617857#comment-17617857
]
ASF GitHub Bot commented on MRESOLVER-278:
------------------------------------------
cstamas commented on code in PR #201:
URL: https://github.com/apache/maven-resolver/pull/201#discussion_r995936078
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java:
##########
@@ -122,50 +114,39 @@ public void initService( final ServiceLocator locator )
public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
{
requireNonNull( session, "session cannot be null" );
- NamedLockFactoryAdapter adapter =
- (NamedLockFactoryAdapter) session.getData().computeIfAbsent(
- ADAPTER_KEY,
- () -> createAdapter( session )
- );
+ NamedLockFactoryAdapter adapter = getOrCreateSessionAdapter( session );
return adapter.newInstance( session, shared );
}
- private NamedLockFactoryAdapter createAdapter( final
RepositorySystemSession session )
+ private NamedLockFactoryAdapter getOrCreateSessionAdapter( final
RepositorySystemSession session )
{
- String nameMapperName = ConfigUtils.getString( session,
DEFAULT_NAME_MAPPER_NAME, NAME_MAPPER_KEY );
- String namedLockFactoryName = ConfigUtils.getString( session,
DEFAULT_FACTORY_NAME, FACTORY_KEY );
- NameMapper nameMapper = nameMappers.get( nameMapperName );
- if ( nameMapper == null )
- {
- throw new IllegalArgumentException( "Unknown NameMapper name: " +
nameMapperName
- + ", known ones: " + nameMappers.keySet() );
- }
- NamedLockFactory namedLockFactory = namedLockFactories.get(
namedLockFactoryName );
- if ( namedLockFactory == null )
+ return (NamedLockFactoryAdapter) session.getData().computeIfAbsent(
ADAPTER_KEY, () ->
{
- throw new IllegalArgumentException( "Unknown NamedLockFactory
name: " + namedLockFactoryName
- + ", known ones: " + namedLockFactories.keySet() );
- }
- NamedLockFactoryAdapter adapter = new NamedLockFactoryAdapter(
nameMapper, namedLockFactory );
- createdAdapters.add( adapter );
- return adapter;
+ String nameMapperName = ConfigUtils.getString( session,
DEFAULT_NAME_MAPPER_NAME, NAME_MAPPER_KEY );
+ String namedLockFactoryName = ConfigUtils.getString( session,
DEFAULT_FACTORY_NAME, FACTORY_KEY );
+ NameMapper nameMapper = nameMappers.get( nameMapperName );
+ if ( nameMapper == null )
+ {
+ throw new IllegalArgumentException( "Unknown NameMapper name:
" + nameMapperName
+ + ", known ones: " + nameMappers.keySet() );
+ }
+ NamedLockFactory namedLockFactory = namedLockFactories.get(
namedLockFactoryName );
+ if ( namedLockFactory == null )
+ {
+ throw new IllegalArgumentException( "Unknown NamedLockFactory
name: " + namedLockFactoryName
+ + ", known ones: " + namedLockFactories.keySet() );
+ }
+ session.addOnCloseHandler( this::shutDownSessionAdapter );
+ return new NamedLockFactoryAdapter( nameMapper, namedLockFactory );
+ } );
}
- @PreDestroy
- public void shutdown()
+ private void shutDownSessionAdapter( RepositorySystemSession session )
{
- LOGGER.debug( "Shutting down created adapters..." );
- createdAdapters.forEach( adapter ->
- {
- try
- {
- adapter.shutdown();
- }
- catch ( Exception e )
- {
- LOGGER.warn( "Could not shutdown: {}", adapter, e );
- }
- }
- );
+ NamedLockFactoryAdapter adapter = (NamedLockFactoryAdapter)
session.getData().get( ADAPTER_KEY );
+ if ( adapter != null )
+ {
+ adapter.shutdown();
+ }
Review Comment:
The adapter shutdown is now handled by session onCloseHandler (as since
https://github.com/apache/maven-resolver/commit/23197f4c50abd8d553059c4f24e2e6d35d015310
commit they are created per-session).
> BREAKING: Make Session extend AutoCloseable (and introduce onCloseHandlers)
> ---------------------------------------------------------------------------
>
> Key: MRESOLVER-278
> URL: https://issues.apache.org/jira/browse/MRESOLVER-278
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
> Reporter: Tamas Cservenak
> Assignee: Tamas Cservenak
> Priority: Major
> Fix For: 1.9.0
>
>
> So far, in (vanilla) Maven, the lifecycle of session was on par with
> lifecycle of SISU container, as Maven does something like this:
> * boot, create container
> * create session
> * work
> * destroy container
> * exit JVM
> So, Maven execution is 1 session 1 container, are on par.
> This is not true for cases where container (and resolver components) are
> reused across several sessions, like mvnd does. Also, current code on master
> (named locks adapter) uses {{@PreDestroy}} to shut down adapters, that is
> invoked when container is shut down, while the adapters are created
> per-session. This means that long-living mvnd daemons will shut down the
> unused adapter only when daemon itself is shut down, even is session for
> which adapter was created is long gone/done.
> While Maven has "session scoped" notion, resolver has not. Hence, simplest
> and cleanest solution is to make RepositorySystemSession have a method to
> "close", denoting that "this session is done". Also, if we can provide hooks
> for "onSessionClose", this resolves all the problems, as for example the
> adapter, that is created per session, could be now cleanly shut down at
> session end.
> One gotcha: this change implies a {*}breaking change for integrators of
> resolver{*}: integrator should make sure they close the session after they
> are done with it.
> Example changes needed for resolver users:
> [https://github.com/apache/maven/pull/822]
> The "pattern" to make use of this feature in resolver is following:
> * stuff something into session data, ideally using computeWhenAbsent
> * if absent, register a callback hook as well
> * in callback, get the stuffed thing from session and if present, do
> something with it
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
