cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227092805
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##########
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
- this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+ this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
Am actually quite uneasy with this "logger level changes at runtime".... As
named lock instances come and go, but this is possible as well:
```
DI container
|-------------------------------------------------------------|
NamedLockFactory (as is singleton)
|-------------------------------------------------------------|
Session1
|-------------------------------|
Session2
| ---------------------|
Session3
| ---------------------|
NamedLock1
|---------------------------------------------------|
NamedLock2
|-------------|
```
As instances are cached (and ref counted), potentially they can span
multiple sessions even, without any connection to session. IF we would always
obey logger level, we could end up with "incomplete" diag data as well, as
logger debug may be disabled-enabled-disabled-enabled, but if lock instance
spans across all 4 changes, diag map would contain only part of data....
--
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]
