From 7bfb429b853927d260b07f06c10ca022dd821676 Mon Sep 17 00:00:00 2001 From: A248 Date: Thu, 25 Aug 2022 13:45:16 -0400 Subject: [PATCH 1/3] Fix handling of futures and exceptions in EconomyProvider --- .../treasury/api/economy/EconomyProvider.java | 74 +++++++++++-------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/api/src/main/java/me/lokka30/treasury/api/economy/EconomyProvider.java b/api/src/main/java/me/lokka30/treasury/api/economy/EconomyProvider.java index 9a47c4f9..c61356cc 100644 --- a/api/src/main/java/me/lokka30/treasury/api/economy/EconomyProvider.java +++ b/api/src/main/java/me/lokka30/treasury/api/economy/EconomyProvider.java @@ -4,8 +4,11 @@ package me.lokka30.treasury.api.economy; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -222,20 +225,12 @@ default void retrieveAllAccountsPlayerIsMemberOf( subscription.succeed(identifiers); return; } - Set ret = new HashSet<>(); + Set ret = Collections.synchronizedSet(new HashSet<>()); + List> futures = new ArrayList<>(identifiers.size()); for (String identifier : identifiers) { - EconomySubscriber.asFuture(subscriber -> retrieveAccount(identifier, - subscriber - )).exceptionally(throwable -> { - if (throwable instanceof EconomyException) { - subscription.fail((EconomyException) throwable); - } else { - subscription.fail(new EconomyException(EconomyFailureReason.OTHER_FAILURE, - throwable - )); - } - return null; - }).thenCompose(account -> { + futures.add(EconomySubscriber.asFuture( + subscriber -> retrieveAccount(identifier, subscriber) + ).thenCompose(account -> { if (account == null) { return CompletableFuture.completedFuture(false); } @@ -250,10 +245,22 @@ default void retrieveAllAccountsPlayerIsMemberOf( if (val) { ret.add(identifier); } - }); - + })); } - subscription.succeed(ret); + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])) + .thenRun(() -> { + subscription.succeed(ret); + }) + .exceptionally((throwable) -> { + if (throwable instanceof EconomyException) { + subscription.fail((EconomyException) throwable); + } else { + subscription.fail(new EconomyException(EconomyFailureReason.OTHER_FAILURE, + throwable + )); + } + return null; + }); }); } @@ -288,20 +295,12 @@ default void retrieveAllAccountsPlayerHasPermission( subscription.succeed(identifiers); return; } - Set ret = new HashSet<>(); + Set ret = Collections.synchronizedSet(new HashSet<>()); + List> futures = new ArrayList<>(identifiers.size()); for (String identifier : identifiers) { - EconomySubscriber.asFuture(subscriber -> retrieveAccount(identifier, - subscriber - )).exceptionally(throwable -> { - if (throwable instanceof EconomyException) { - subscription.fail((EconomyException) throwable); - } else { - subscription.fail(new EconomyException(EconomyFailureReason.OTHER_FAILURE, - throwable - )); - } - return null; - }).thenCompose(account -> { + futures.add(EconomySubscriber.asFuture( + subscriber -> retrieveAccount(identifier, subscriber) + ).thenCompose(account -> { if (account == null) { return CompletableFuture.completedFuture(false); } @@ -327,9 +326,22 @@ public void fail(@NotNull final EconomyException exception) { if (val) { ret.add(identifier); } - }); + })); } - subscription.succeed(ret); + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])) + .thenRun(() -> { + subscription.succeed(ret); + }) + .exceptionally((throwable) -> { + if (throwable instanceof EconomyException) { + subscription.fail((EconomyException) throwable); + } else { + subscription.fail(new EconomyException(EconomyFailureReason.OTHER_FAILURE, + throwable + )); + } + return null; + }); }); } From 242bcf4b4ace0e51cc6daff1d389a8cf69b5080a Mon Sep 17 00:00:00 2001 From: A248 Date: Thu, 25 Aug 2022 13:56:35 -0400 Subject: [PATCH 2/3] Use ExecutorService#execute to avoid swallowing exceptions It should not happen, but in the rare case it does, at least an exception will be logged. --- .../java/me/lokka30/treasury/api/common/event/EventBus.java | 2 +- .../java/me/lokka30/treasury/api/common/event/EventCaller.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/me/lokka30/treasury/api/common/event/EventBus.java b/api/src/main/java/me/lokka30/treasury/api/common/event/EventBus.java index 63834b6a..b96ae1b5 100644 --- a/api/src/main/java/me/lokka30/treasury/api/common/event/EventBus.java +++ b/api/src/main/java/me/lokka30/treasury/api/common/event/EventBus.java @@ -70,7 +70,7 @@ public FireCompletion fire(@NotNull T event) { List> friends = eventTypes.getFriendsOf(event.getClass()); ExecutorService async = EventExecutorTracker.INSTANCE.getExecutor(event.getClass()); FireCompletion ret = new FireCompletion<>(event.getClass()); - async.submit(() -> { + async.execute(() -> { List completions = new ArrayList<>(); EventCaller caller = events.get(event.getClass()); if (caller != null) { diff --git a/api/src/main/java/me/lokka30/treasury/api/common/event/EventCaller.java b/api/src/main/java/me/lokka30/treasury/api/common/event/EventCaller.java index 9ec1f002..9960ce85 100644 --- a/api/src/main/java/me/lokka30/treasury/api/common/event/EventCaller.java +++ b/api/src/main/java/me/lokka30/treasury/api/common/event/EventCaller.java @@ -28,7 +28,7 @@ public Completion call(Object event) { return Completion.completed(); } Completion completion = new Completion(); - EventExecutorTracker.INSTANCE.getExecutor(eventClass).submit(() -> { + EventExecutorTracker.INSTANCE.getExecutor(eventClass).execute(() -> { List errors = call(event, new ArrayList<>(), 0); if (!errors.isEmpty()) { completion.completeExceptionally(errors); From 6690f8206455909ba2450d75cb9c2c3135e35252 Mon Sep 17 00:00:00 2001 From: A248 Date: Thu, 25 Aug 2022 14:01:45 -0400 Subject: [PATCH 3/3] Make EventTypeTracker thread-safe + some code cleanup Remove unnecessary collect() and re-stream() operation --- .../api/common/event/EventTypeTracker.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/api/src/main/java/me/lokka30/treasury/api/common/event/EventTypeTracker.java b/api/src/main/java/me/lokka30/treasury/api/common/event/EventTypeTracker.java index d02e12e6..c0dc1a87 100644 --- a/api/src/main/java/me/lokka30/treasury/api/common/event/EventTypeTracker.java +++ b/api/src/main/java/me/lokka30/treasury/api/common/event/EventTypeTracker.java @@ -6,37 +6,32 @@ import com.google.common.reflect.TypeToken; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +import java.util.stream.Stream; class EventTypeTracker { - private Map, List>> friends = new HashMap<>(); + private final Map, List>> friends = new ConcurrentHashMap<>(); - public List> getFriendsOf(Class eventType) { - if (friends.containsKey(eventType)) { - return Collections.unmodifiableList(friends.get(eventType)); - } - - List> types = getEventTypes(eventType); - friends.put( - eventType, - types.stream().filter(type -> type != eventType).collect(Collectors.toList()) - ); - - return friends.get(eventType); + public List> getFriendsOf(Class event) { + List> computedFriends = friends.computeIfAbsent((event), (eventType) -> { + return getEventTypes(eventType) + .filter(type -> type != eventType) + .collect(Collectors.toList()); + }); + return Collections.unmodifiableList(computedFriends); } - private static List> getEventTypes(Class eventType) { + private static Stream> getEventTypes(Class eventType) { return TypeToken .of(eventType) .getTypes() .rawTypes() .stream() - .filter(type -> type != Object.class && type != Cancellable.class) - .collect(Collectors.toList()); + .filter(type -> type != Object.class && type != Cancellable.class); } }