Add support for retry operations to help aleviate #193. Also, fix the…

… bug with preference updates only taking effect on project start.

CHANGES.md

@@ -1,6 +1,24 @@
 # IDEA Community VCS Integration for Perforce
 
 
+## ::v0.10.2::
+
+### Overview
+
+* Added retry connection on socket timeout.
+* Bug fixes
+
+### Details
+
+* Added retry connection on socket timeout.
+    * Even with an extended socket timeout, the server connection would sometimes
+      fail. (#193)
+* Bug fixes
+    * When the socket timeout or lock timeout change, the new value would
+      not take effect until the project is reopened.  This should now take effect
+      soon after setting the value. 
+
+
 ## ::v0.10.1::
 
 ### Overview

idea-p4server/api/src/main/java/net/groboclown/p4/server/api/messagebus/UserProjectPreferencesUpdatedMessage.java

@@ -0,0 +1,60 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.groboclown.p4.server.api.messagebus;
+
+import com.intellij.openapi.project.Project;
+import com.intellij.util.messages.Topic;
+import org.jetbrains.annotations.NotNull;
+
+public class UserProjectPreferencesUpdatedMessage
+        extends ProjectMessage<UserProjectPreferencesUpdatedMessage.Listener> {
+    private static final String DISPLAY_NAME = "p4ic4idea:user preferences updated";
+    private static final Topic<Listener> TOPIC = new Topic<>(
+            DISPLAY_NAME, Listener.class, Topic.BroadcastDirection.TO_CHILDREN
+    );
+    private static final Listener DEFAULT_LISTENER = new ListenerAdapter();
+
+    public static class UserPreferencesUpdatedEvent extends AbstractMessageEvent {
+        // Note that the project settings are not passed into this class, because
+        // it's not in scope for this class!
+        public UserPreferencesUpdatedEvent() {
+        }
+    }
+
+    public interface Listener {
+        void userPreferencesUpdated(@NotNull UserPreferencesUpdatedEvent e);
+    }
+
+    public static class ListenerAdapter implements Listener {
+        @Override
+        public void userPreferencesUpdated(@NotNull UserPreferencesUpdatedEvent e) {
+        }
+    }
+
+    /**
+     * Should only be called by {@link net.groboclown.p4.server.api.ProjectConfigRegistry}.  Doesn't conform to
+     * the "send(Project)" standard API, because it can trigger 2 events.
+     *
+     * @param project project to send the message on.
+     */
+    public static Listener send(@NotNull Project project) {
+        return getListener(project, TOPIC, DEFAULT_LISTENER);
+    }
+
+    public static void addListener(@NotNull MessageBusClient.ProjectClient client,
+            @NotNull Object listenerOwner, @NotNull Listener listener) {
+        addListener(client, TOPIC, listener, Listener.class, listenerOwner);
+    }
+}

idea-p4server/api/src/main/java/net/groboclown/p4/server/api/messagebus/UserSelectedOfflineMessage.java

@@ -19,36 +19,39 @@
 import net.groboclown.p4.server.api.P4ServerName;
 import org.jetbrains.annotations.NotNull;
 
-public class UserSelectedOfflineMessage extends ProjectMessage<UserSelectedOfflineMessage.Listener> {
-private static final String DISPLAY_NAME = "p4ic4idea:user selected a server to go offline";
-private static final Topic<Listener> TOPIC = new Topic<>(
-        DISPLAY_NAME,
-        Listener.class,
-        Topic.BroadcastDirection.TO_CHILDREN);
-private static final Listener DEFAULT_LISTENER = new ListenerAdapter();
-
-public static class OfflineEvent extends AbstractMessageEvent {
-    private final P4ServerName name;
-
-    public OfflineEvent(@NotNull P4ServerName name) {
-        this.name = name;
-    }
+public class UserSelectedOfflineMessage
+        extends ProjectMessage<UserSelectedOfflineMessage.Listener> {
+    private static final String DISPLAY_NAME = "p4ic4idea:user selected a server to go offline";
+    private static final Topic<Listener> TOPIC = new Topic<>(
+            DISPLAY_NAME,
+            Listener.class,
+            Topic.BroadcastDirection.TO_CHILDREN);
+    private static final Listener DEFAULT_LISTENER = new ListenerAdapter();
+
+    public static class OfflineEvent
+            extends AbstractMessageEvent {
+        private final P4ServerName name;
 
-    @NotNull
-    public P4ServerName getName() {
-        return name;
+        public OfflineEvent(@NotNull P4ServerName name) {
+            this.name = name;
+        }
+
+        @NotNull
+        public P4ServerName getName() {
+            return name;
+        }
     }
-}
 
-public interface Listener {
-    void userSelectedServerOffline(@NotNull OfflineEvent e);
-}
+    public interface Listener {
+        void userSelectedServerOffline(@NotNull OfflineEvent e);
+    }
 
-public static class ListenerAdapter implements Listener {
-    @Override
-    public void userSelectedServerOffline(@NotNull OfflineEvent e) {
+    public static class ListenerAdapter
+            implements Listener {
+        @Override
+        public void userSelectedServerOffline(@NotNull OfflineEvent e) {
+        }
     }
-}
 
 
     public static Listener send(@NotNull Project project) {

idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/P4RequestErrorHandler.java

@@ -113,6 +113,16 @@ protected abstract P4CommandRunner.ServerResultException handleException(
     protected abstract P4CommandRunner.ServerResultException handleError(
             @NotNull ConnectionInfo info, @NotNull Error e);
 
+    /**
+     * Some exceptions may be an issue with latency to the server, and should be retried a certain number of times.
+     *
+     * @param e encountered exception
+     * @return true if the error should be retried, false if not.
+     */
+    protected abstract boolean isRetryableError(@NotNull Exception e);
+
+    protected abstract int getMaxRetryCount();
+
 
     @NotNull
     protected final P4CommandRunner.ServerResultException createServerResultException(@Nullable Exception e,
@@ -131,18 +141,33 @@ protected final P4CommandRunner.ServerResultException createServerResultExceptio
     }
 
 
-
     @Nullable
     private <R> R handleConnection(@NotNull ConnectionInfo info, @NotNull Callable<R> c)
             throws P4CommandRunner.ServerResultException {
-        try {
-            return c.call();
-        } catch (Exception e) {
-            throw handleException(info, e);
-        } catch (VirtualMachineError | ThreadDeath e) {
-            throw e;
-        } catch (Error e) {
-            throw handleError(info, e);
+        final int maxRetryCount = getMaxRetryCount();
+
+        // Tricky logic.  The block will only loop if an error is retry-able AND the retry count is below the max.
+        // Every other branch is a return.
+        // See #193 for an explanation for the need for retry.
+        // Note that, the way this is implemented, a max retry count <= 0 will still run, which is as
+        // expected.
+        for (int retryCount = 0; true; retryCount++) {
+            try {
+                return c.call();
+            } catch (Exception e) {
+                if (isRetryableError(e) && retryCount < maxRetryCount) {
+                    continue;
+                }
+                throw handleException(info, e);
+            } catch (VirtualMachineError | ThreadDeath e) {
+                // Don't ever process or swallow these.
+                throw e;
+            } catch (Error e) {
+                throw handleError(info, e);
+            } catch (Throwable t) {
+                LOG.error("Unexpected generic throwable", t);
+                throw handleException(info, new RuntimeException(t));
+            }
         }
     }
 

idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/LimitedConnectionManager.java

@@ -49,6 +49,10 @@ public LimitedConnectionManager(@NotNull ConnectionManager proxy, int maximumCon
         this.proxy = proxy;
     }
 
+    public void setLockTimeout(long timeout, @NotNull TimeUnit timeoutUnit) {
+        this.restriction.setTimeout(timeout, timeoutUnit);
+    }
+
     @NotNull
     @Override
     public <R> Answer<R> withConnection(@NotNull ClientConfig config, @NotNull P4Func<IClient, R> fun) {

idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/MessageP4RequestErrorHandler.java

@@ -62,6 +62,7 @@
 import javax.annotation.Nonnull;
 import javax.crypto.Cipher;
 import java.io.IOException;
+import java.net.SocketTimeoutException;
 import java.net.URISyntaxException;
 import java.net.UnknownHostException;
 import java.security.NoSuchAlgorithmException;
@@ -455,6 +456,11 @@ protected P4CommandRunner.ServerResultException handleException(
                     getMessage("error.P4JavaException", e),
                     P4CommandRunner.ErrorCategory.SERVER_ERROR);
         }
+        if (e instanceof SocketTimeoutException) {
+            // see #193
+            ConnectionErrorMessage.send().connectionError(new ServerErrorEvent.ServerNameErrorEvent<>(
+                    info.getServerName(), info.getServerConfig(), new ConnectionException(e)));
+        }
         if (e instanceof IOException) {
             // This shouldn't be a server connection issue, because those are wrapped in ConnectionException classes.
             // That just leaves local file I/O problems.
@@ -515,6 +521,26 @@ protected P4CommandRunner.ServerResultException handleException(
                 P4CommandRunner.ErrorCategory.INTERNAL);
     }
 
+    protected boolean isRetryableError(@NotNull Exception e) {
+        Throwable next = e;
+        Throwable prev = null;
+        while (next != null && prev != next) {
+            // See #192 - SocketTimeoutException can be a sign that we need to retry the operation.
+            if (next instanceof SocketTimeoutException) {
+                return true;
+            }
+            prev = next;
+            next = next.getCause();
+        }
+        return false;
+    }
+
+
+    protected final Project getProject() {
+        return project;
+    }
+
+
     private static boolean isUnlimitedStrengthEncryptionInstalled() {
         try {
             return Cipher.getMaxAllowedKeyLength("RC5") >= 256;

idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/SimpleConnectionManager.java

@@ -69,7 +69,7 @@ public class SimpleConnectionManager implements ConnectionManager {
     private static final char[] EMPTY_PASSWORD = new char[0];
 
     private final File tmpDir;
-    private final int socketSoTimeoutMillis;
+    private int socketSoTimeoutMillis;
     private final String pluginVersion;
     private final P4RequestErrorHandler errorHandler;
 
@@ -82,6 +82,12 @@ public SimpleConnectionManager(File tmpDir, int socketSoTimeoutMillis, String pl
     }
 
 
+    public void setSocketSoTimeoutMillis(int socketSoTimeoutMillis) {
+        this.socketSoTimeoutMillis = socketSoTimeoutMillis;
+        // If this ever caches the connection, then the connection should be closed here.
+    }
+
+
     @NotNull
     @Override
     public <R> Answer<R> withConnection(@NotNull ClientConfig config, @Nonnull P4Func<IClient, R> fun) {
@@ -187,7 +193,7 @@ private IOptionsServer connect(ServerConfig serverConfig, String password, Prope
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Setting up SSO login to execute `" + serverConfig.getLoginSso() + "`");
             }
-            // FIXME pass in a error callback to the callback so that the connection manager can handle errors.
+            // pass in a error callback to the callback so that the connection manager can handle errors.
             // TODO look into registering the sso key through user options.
             server.registerSSOCallback(new LoginSsoCallbackHandler(serverConfig, 1000), null);
         }
@@ -205,7 +211,7 @@ private IOptionsServer connect(ServerConfig serverConfig, String password, Prope
         // set, then a simple password login attempt should be made.  The P4LOGINSSO will
         // ignore the password.
         // However, we will always perform the SSO login here.
-        // FIXME with the addition of the LoginAction, we should only perform this when absolutely required.
+        // With the addition of the LoginAction, we should only perform this when absolutely required.
         // The original server authentication terribleness is enshrined in v2's ServerAuthenticator.
         if (serverConfig.hasLoginSso() || (serverConfig.usesStoredPassword() && password != null)) {
             final boolean useTicket = serverConfig.getAuthTicket() != null && serverConfig.getAuthTicket().isFile()

idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/TraceableSemaphore.java

@@ -36,19 +36,31 @@ public class TraceableSemaphore {
 
     private static final AtomicInteger ID_GEN = new AtomicInteger();
     private final String name;
-    private final long timeout;
-    private final TimeUnit timeoutUnit;
+
+    // Access to these two values must be synchronized on "name".
+    private long timeout;
+    private TimeUnit timeoutUnit;
+
     private final Semaphore sem;
     private final List<Requester> pending = Collections.synchronizedList(new ArrayList<>());
     private final List<Requester> active = Collections.synchronizedList(new ArrayList<>());
 
-    public TraceableSemaphore(String name, int maximumConcurrentCount, long timeout, TimeUnit timeoutUnit) {
+    public TraceableSemaphore(String name, int maximumConcurrentCount, long timeout, @NotNull TimeUnit timeoutUnit) {
         this.name = name;
-        this.timeout = timeout;
-        this.timeoutUnit = timeoutUnit;
+        setTimeout(timeout, timeoutUnit);
         this.sem = new Semaphore(maximumConcurrentCount);
     }
 
+    public void setTimeout(long timeout, @NotNull TimeUnit timeoutUnit) {
+        if (timeout <= 0) {
+            throw new IllegalArgumentException("bad timeout");
+        }
+        synchronized (name) {
+            this.timeout = timeout;
+            this.timeoutUnit = timeoutUnit;
+        }
+    }
+
     public static Requester createRequest() {
         int id = ID_GEN.incrementAndGet();
         String name = getRequestName();
@@ -88,16 +100,25 @@ public void acquire(@NotNull Requester req) throws InterruptedException, Cancell
             pending.add(req);
             LOG.debug(name + ": WAIT - wait queue = " + pending + "; active = " + active);
         }
+
+        final long usableTimeout;
+        final TimeUnit usableTimeoutUnit;
+        synchronized (name) {
+            usableTimeout = this.timeout;
+            usableTimeoutUnit = this.timeoutUnit;
+        }
+
         try {
-            boolean captured = sem.tryAcquire(timeout, timeoutUnit);
+            boolean captured = sem.tryAcquire(usableTimeout, usableTimeoutUnit);
             if (LOG.isDebugEnabled()) {
                 pending.remove(req);
             }
             if (!captured) {
                 if (LOG.isDebugEnabled()) {
                     LOG.debug(name + ": TIMEOUT - wait queue = " + pending + "; active = " + active);
                 }
-                throw new CancellationException("Waiting for lock timed out: exceeded " + timeout + " " + timeoutUnit.toString().toLowerCase());
+                throw new CancellationException("Waiting for lock timed out: exceeded " + usableTimeout + " " +
+                        usableTimeoutUnit.toString().toLowerCase());
             }
             req.activate();
             active.add(req);

idea-p4server/impl/src/test/java/net/groboclown/p4/server/impl/connection/FailP4RequestErrorHandler.java

@@ -42,6 +42,16 @@ protected P4CommandRunner.ServerResultException handleError(@NotNull ConnectionI
         return null;
     }
 
+    @Override
+    protected boolean isRetryableError(@NotNull Exception e) {
+        return false;
+    }
+
+    @Override
+    protected int getMaxRetryCount() {
+        return 0;
+    }
+
 
     @Override
     public void handleOnDisconnectError(@Nonnull ConnectionException e) {