gnodet-bot commented on code in PR #13602:
URL: https://github.com/apache/maven/pull/13602#discussion_r4304962532
##########
impl/maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java:
##########
@@ -515,6 +561,91 @@ public T get() {
}
}
+ /**
+ * Captures a human-readable snapshot of the current native registry for
Maven internals,
+ * including direct binding map and metadata changes, without resolving
components.
+ * Registration must not run concurrently with this method.
+ */
+ public String dumpBindings() {
+ return BindingSnapshot.render(
+ this, registryRevision, "explicit", false,
BindingSnapshot.capture(bindings, scopes));
Review Comment:
💡 **`dumpBindings()` lacks the diagnostic lock that `writeBindings()` holds**
`writeBindings()` (the internal diagnostic path) acquires `diagnosticLock`
before calling `BindingSnapshot.capture(bindings, scopes)`. The public
`dumpBindings()` calls the same `capture()` without any lock. The Javadoc says
"Registration must not run concurrently", but callers have no compile-time
enforcement and the inconsistency in locking discipline could surprise future
maintainers.
At minimum, document the weaker guarantee explicitly:
```suggestion
/**
* Captures a human-readable snapshot of the current native registry for
Maven internals,
* including direct binding map and metadata changes, without resolving
components.
* <p>
* <strong>Thread safety:</strong> this method reads the binding map and
scope map
* without internal synchronization. The caller must ensure no
registration is in
* progress, or accept a best-effort snapshot.
*/
public String dumpBindings() {
return BindingSnapshot.render(
this, registryRevision, "explicit", false,
BindingSnapshot.capture(bindings, scopes));
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
