[
https://issues.apache.org/jira/browse/MRESOLVER-278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17617790#comment-17617790
]
ASF GitHub Bot commented on MRESOLVER-278:
------------------------------------------
gnodet commented on code in PR #201:
URL: https://github.com/apache/maven-resolver/pull/201#discussion_r995866800
##########
maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java:
##########
@@ -271,4 +272,57 @@
@Deprecated
FileTransformerManager getFileTransformerManager();
+ /**
+ * Adds "on close" handler to this session, it must not be {@code null}.
Note: when handlers are invoked, the
+ * passed in (this) session is ALREADY CLOSED (the {@link #isClosed()}
method returns {@code true}). This implies,
+ * that handlers cannot use {@link RepositorySystem} to
resolve/collect/and so on, handlers are meant to perform
+ * some internal cleanup on session close. Attempt to add handler to
closed session will throw.
+ *
+ * @since TBD
+ */
+ void addOnCloseHandler( Consumer<RepositorySystemSession> handler );
+
+ /**
+ * Returns {@code true} if this instance was already closed. Closed
sessions should NOT be used anymore.
+ *
+ * @return {@code true} if session was closed.
+ * @since TBD
+ */
+ boolean isClosed();
+
+ /**
+ * Closes this session and possibly releases related resources by invoking
"on close" handlers added by
+ * {@link #addOnCloseHandler(Consumer<RepositorySystemSession>)} method.
This method may be invoked multiple times,
+ * but only first invocation will actually invoke handlers, subsequent
invocations will be no-op.
+ * <p>
+ * When close action happens, all the registered handlers will be invoked
(even if some throws), and at end
+ * of operation a {@link MultiRuntimeException} may be thrown signaling
that some handler(s) failed. This exception
+ * may be ignored, is at the discretion of caller.
+ * <p>
+ * Important: ideally it is the session creator who should be responsible
to close the session. The "nested"
+ * (delegating, wrapped) sessions {@link
AbstractForwardingRepositorySystemSession} and alike) by default
+ * (without overriding the {@link
AbstractForwardingRepositorySystemSession#close()} method) are prevented to
close
+ * session, and it is the "recommended" behaviour as well. On the other
hand, "nested" session may receive new
+ * "on close" handler registrations, but those registrations are passed to
delegated session, and will be invoked
+ * once the "top level" (delegated) session is closed.
+ * <p>
+ * New session "copy" instances created using copy-constructor
+ * {@link
DefaultRepositorySystemSession#DefaultRepositorySystemSession(RepositorySystemSession)}
result in new,
+ * independent session instances, and they do NOT share states like
"read-only", "closed" and "on close handlers"
+ * with the copied session. Hence, they should be treated as "top level"
sessions as well.
+ * <p>
+ * The recommended pattern for "top level" sessions is the usual
try-with-resource:
+ *
+ * <pre> {@code
+ * try ( RepositorySystemSession session = create session... )
Review Comment:
I disagree, the scope is well defined in maven:
https://github.com/apache/maven/blob/ae655e0e83e47ec7389763bcabe9d84e92862179/maven-core/src/main/java/org/apache/maven/DefaultMaven.java#L234-L253
> 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)
