From f99cd23c1328d6cbdbf126e46bd628912c31d0ab Mon Sep 17 00:00:00 2001 From: mizosoft Date: Sat, 10 Feb 2024 23:35:52 +0200 Subject: [PATCH 1/2] Use checked exception --- .../methanol/testing/store/RedisSupport.java | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/methanol-testing/src/main/java/com/github/mizosoft/methanol/testing/store/RedisSupport.java b/methanol-testing/src/main/java/com/github/mizosoft/methanol/testing/store/RedisSupport.java index edc1bdbf8..ce3954a73 100644 --- a/methanol-testing/src/main/java/com/github/mizosoft/methanol/testing/store/RedisSupport.java +++ b/methanol-testing/src/main/java/com/github/mizosoft/methanol/testing/store/RedisSupport.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Moataz Abdelnasser + * Copyright (c) 2024 Moataz Abdelnasser * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -32,6 +32,8 @@ import java.lang.System.Logger.Level; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -42,7 +44,7 @@ class RedisSupport { static final String CLI_CMD = "redis-cli"; @GuardedBy("RedisSupport.class") - private static @MonotonicNonNull Set lazyAvailability = null; + private static @MonotonicNonNull Set lazyAvailableCommands = null; private RedisSupport() {} @@ -57,12 +59,12 @@ public static boolean isRedisClusterAvailable() { private static synchronized boolean isAvailable(String command) { requireArgument( command.equals(SERVER_CMD) || command.equals(CLI_CMD), "unrecognized command: %s", command); - var availability = lazyAvailability; - if (availability == null) { - availability = checkAvailability(); - lazyAvailability = availability; + var availableCommands = lazyAvailableCommands; + if (availableCommands == null) { + availableCommands = checkAvailability(); + lazyAvailableCommands = availableCommands; } - return availability.contains(command); + return availableCommands.contains(command); } private static Set checkAvailability() { @@ -88,7 +90,7 @@ private static Set checkAvailability() { } } - private static String versionOf(String command) { + private static String versionOf(String command) throws UnavailableCommandException { try { var process = new ProcessBuilder().command(command, "--version").redirectErrorStream(true).start(); @@ -113,7 +115,8 @@ private static void reportUnavailability( String cmd, String unavailabilityReason, @Nullable Throwable exception, - @Nullable Reader reader) { + @Nullable Reader reader) + throws UnavailableCommandException { String output; try { if (reader != null) { @@ -131,7 +134,13 @@ private static void reportUnavailability( } throw new UnavailableCommandException( - String.format("Unavailable redis command <%s>: %s%n%s", cmd, unavailabilityReason, output), + String.format( + "Unavailable redis command <%s>: %s%n%s", + cmd, + unavailabilityReason, + Stream.of(output.split("\\r?\\n")) + .map(line -> "\t" + line) + .collect(Collectors.joining())), exception); } @@ -145,7 +154,7 @@ static String dumpRemaining(Reader reader) throws IOException { return sb.toString(); } - private static final class UnavailableCommandException extends IllegalStateException { + private static final class UnavailableCommandException extends IOException { UnavailableCommandException(String message, Throwable cause) { super(message, cause); } From 4632bf134ea2f0b20219292c439627222075b3bd Mon Sep 17 00:00:00 2001 From: mizosoft Date: Sat, 10 Feb 2024 23:36:15 +0200 Subject: [PATCH 2/2] Fix not accessing 'ongoing' via its lock (leftover from prev implementation) --- .../extensions/HttpResponsePublisher.java | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/methanol/src/main/java/com/github/mizosoft/methanol/internal/extensions/HttpResponsePublisher.java b/methanol/src/main/java/com/github/mizosoft/methanol/internal/extensions/HttpResponsePublisher.java index 7d8fc5fa0..88cd7e939 100644 --- a/methanol/src/main/java/com/github/mizosoft/methanol/internal/extensions/HttpResponsePublisher.java +++ b/methanol/src/main/java/com/github/mizosoft/methanol/internal/extensions/HttpResponsePublisher.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Moataz Abdelnasser + * Copyright (c) 2024 Moataz Abdelnasser * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -27,8 +27,6 @@ import com.github.mizosoft.methanol.internal.flow.AbstractQueueSubscription; import com.github.mizosoft.methanol.internal.flow.FlowSupport; import com.google.errorprone.annotations.concurrent.GuardedBy; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.VarHandle; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; @@ -86,17 +84,6 @@ public void subscribe(Subscriber> subscriber) { private static final class SubscriptionImpl extends AbstractQueueSubscription> { - private static final VarHandle ONGOING; - - static { - try { - var lookup = MethodHandles.lookup(); - ONGOING = lookup.findVarHandle(SubscriptionImpl.class, "ongoing", int.class); - } catch (IllegalAccessException | NoSuchFieldException e) { - throw new ExceptionInInitializerError(e); - } - } - private final HttpClient client; private final HttpRequest initialRequest; private final BodyHandler handler; @@ -238,7 +225,13 @@ private void applyPushPromise( } if (pushedResponseBodyHandler != null) { - ONGOING.getAndAdd(SubscriptionImpl.this, 1); + lock.lock(); + try { + ongoing += 1; + } finally { + lock.unlock(); + } + acceptor.apply(pushedResponseBodyHandler).whenComplete(SubscriptionImpl.this::onResponse); } }