gnodet commented on code in PR #1902:
URL: https://github.com/apache/maven-resolver/pull/1902#discussion_r3371903488
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/SmartExecutor.java:
##########
@@ -84,32 +85,44 @@ class Pooled implements SmartExecutor {
@Override
public void submit(Runnable runnable) {
ClassLoader tccl = Thread.currentThread().getContextClassLoader();
- executor.submit(() -> {
- ClassLoader old =
Thread.currentThread().getContextClassLoader();
- Thread.currentThread().setContextClassLoader(tccl);
- try {
- runnable.run();
- } finally {
- Thread.currentThread().setContextClassLoader(old);
- }
- });
+ try {
+ executor.submit(() -> {
+ ClassLoader old =
Thread.currentThread().getContextClassLoader();
+ Thread.currentThread().setContextClassLoader(tccl);
+ try {
+ runnable.run();
+ } finally {
+ Thread.currentThread().setContextClassLoader(old);
+ }
+ });
+ } catch (RejectedExecutionException e) {
+ runnable.run();
+ }
Review Comment:
Fixed. The fallback `runnable.run()` is now wrapped in a
`try-catch(RuntimeException | Error)` that swallows the exception. This
preserves the async `submit()` semantics where exceptions are captured by the
Future. Callers like `RunnableErrorForwarder` already record the error before
re-throwing, so it will still be reported via `await()`.
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/SmartExecutor.java:
##########
@@ -136,13 +149,21 @@ class Limited implements SmartExecutor {
public void submit(Runnable runnable) {
try {
semaphore.acquire();
- executor.submit(() -> {
+ try {
+ executor.submit(() -> {
+ try {
+ runnable.run();
+ } finally {
+ semaphore.release();
+ }
+ });
+ } catch (RejectedExecutionException e) {
try {
runnable.run();
} finally {
semaphore.release();
Review Comment:
Fixed. Same approach as the `Pooled` case above — the fallback
`runnable.run()` in `Limited.submit(Runnable)` now catches and swallows
`RuntimeException | Error` to match async submission semantics. The semaphore
is still properly released in the `finally` block.
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/CachingArtifactTypeRegistry.java:
##########
@@ -45,17 +45,18 @@ public static ArtifactTypeRegistry
newInstance(ArtifactTypeRegistry delegate) {
private CachingArtifactTypeRegistry(ArtifactTypeRegistry delegate) {
this.delegate = delegate;
- types = new HashMap<>();
+ types = new ConcurrentHashMap<>();
}
public ArtifactType get(String typeId) {
ArtifactType type = types.get(typeId);
-
- if (type == null) {
- type = delegate.get(typeId);
+ if (type != null) {
+ return type;
+ }
Review Comment:
Fixed. Replaced the get-then-delegate-then-put sequence with
`types.computeIfAbsent(typeId, delegate::get)` for atomic cache population.
This avoids redundant `delegate.get()` calls under contention.
##########
maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/GnupgSignatureArtifactGenerator.java:
##########
@@ -63,13 +65,13 @@ final class GnupgSignatureArtifactGenerator implements
ArtifactGenerator {
PGPPrivateKey privateKey,
PGPSignatureSubpacketVector hashSubPackets,
String keyInfo) {
- this.artifacts = new ArrayList<>(artifacts);
+ this.artifacts = new CopyOnWriteArrayList<>(artifacts);
this.signableArtifactPredicate = signableArtifactPredicate;
this.secretKey = secretKey;
this.privateKey = privateKey;
this.hashSubPackets = hashSubPackets;
this.keyInfo = keyInfo;
- this.signatureTempFiles = new ArrayList<>();
+ this.signatureTempFiles = new CopyOnWriteArrayList<>();
logger.debug("Created generator using key {}", keyInfo);
Review Comment:
Fixed. Replaced `CopyOnWriteArrayList` with plain `ArrayList` for both
`artifacts` and `signatureTempFiles`, and made `generate()` and `close()`
`synchronized`. This avoids the array-copy overhead on each write while still
providing thread safety and happens-before guarantees between the two methods.
--
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]
