Move retry logic into RMIRegistryEndpoint

qtc-de · Jun 9, 2024 · d29118f · d29118f
1 parent bbbdb9f
commit d29118f
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 38 deletions.
diff --git a/src/eu/tneitzel/rmg/networking/RMIRegistryEndpoint.java b/src/eu/tneitzel/rmg/networking/RMIRegistryEndpoint.java
@@ -18,6 +18,7 @@
 import eu.tneitzel.rmg.internal.RMGOption;
 import eu.tneitzel.rmg.io.Logger;
 import eu.tneitzel.rmg.plugin.PluginSystem;
+import eu.tneitzel.rmg.utils.EmptyWrapper;
 import eu.tneitzel.rmg.utils.RMGUtils;
 import eu.tneitzel.rmg.utils.RemoteObjectWrapper;
 
@@ -280,6 +281,42 @@ else if( cause instanceof SSRFException )
         return remoteObject;
     }
 
+    /**
+     * It was observed that using --serial-version-uid option can cause an invalid transport return code
+     * exception during lookup. This seems to be some kind of race condition and cannot be reproduced reliably.
+     * We currently believe that RMI / Java does not clear the ObjectInput stream when reading an unknown class
+     * from it. The remaining bytes are left within the stream. Since RMI uses connection pooling, the next
+     * operation encounters the invalid bytes and fails. If this is the case, we just retry a few times.
+     *
+     * @param boundName  the bound name to lookup
+     * @param maxRetries  the maximum amount of retries to perform
+     * @return Remote object if lookup was successful. null otherwise.
+     */
+    public Remote lookupWithRetries(String boundName, int maxRetries)
+    {
+        int retryCount = 0;
+
+        while (retryCount < maxRetries)
+        {
+            try
+            {
+                return this.lookup(boundName);
+            }
+
+            catch (java.rmi.UnmarshalException e)
+            {
+                retryCount += 1;
+            }
+
+            catch (Exception e)
+            {
+                ExceptionHandler.unexpectedException(e, "lookup", "operation", true);
+            }
+        }
+
+        return null;
+    }
+
     /**
      * Same as the lookup action, but returns a RemoteObjectWrapper.
      *
@@ -291,10 +328,21 @@ else if( cause instanceof SSRFException )
      * @throws SecurityException if reflective access fails
      * @throws UnmarshalException if unmarshalling the return value fails
      */
-    public RemoteObjectWrapper lookupWrapper(String boundName) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException, UnmarshalException
+    public RemoteObjectWrapper lookupWrapper(String boundName)
     {
-        Remote remoteObject = lookup(boundName);
-        return RemoteObjectWrapper.getInstance(remoteObject, boundName);
+        Remote remoteObject = lookupWithRetries(boundName, 5);
+
+        if (remoteObject != null)
+        {
+            try
+            {
+                return RemoteObjectWrapper.getInstance(remoteObject, boundName);
+            }
+
+            catch (IllegalArgumentException | IllegalAccessException | NoSuchFieldException | SecurityException e) {}
+        }
+
+        return new EmptyWrapper(boundName);
     }
 
     /**
@@ -308,7 +356,7 @@ public RemoteObjectWrapper lookupWrapper(String boundName) throws IllegalArgumen
      * @throws SecurityException if reflective access fails
      * @throws UnmarshalException if unmarshalling the return value fails
      */
-    public RemoteObjectWrapper[] lookupWrappers(String[] boundNames) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException, UnmarshalException
+    public RemoteObjectWrapper[] lookupWrappers(String[] boundNames)
     {
         RemoteObjectWrapper[] wrappers = new RemoteObjectWrapper[boundNames.length];
 

diff --git a/src/eu/tneitzel/rmg/operations/Dispatcher.java b/src/eu/tneitzel/rmg/operations/Dispatcher.java
@@ -89,12 +89,6 @@ private void obtainBoundNames() throws NoSuchObjectException
      * was specified on the command line, all registered bound names within the RMI registry are looked up.
      * The result is stored within an object attribute.
      *
-     * It was observed that using --serial-version-uid option can cause an invalid transport return code
-     * exception. This seems to be some kind of race condition and cannot be reproduced reliably. It seems
-     * that RMI / Java does not clear the ObjectInput stream when reading an unknown class from it. The remaining
-     * bytes are left within the stream. Since RMI uses connection pooling, the next operation encounteres the
-     * invalid bytes and fails. If this is the case, we just retry a few times.
-     *
      * @throws java.rmi.NoSuchObjectException is thrown when the specified RMI endpoint is not an RMI registry
      */
     private void obtainBoundObjects() throws NoSuchObjectException
@@ -123,34 +117,7 @@ private void obtainBoundObjects() throws NoSuchObjectException
                 obtainBoundNames();
             }
 
-            remoteObjects = new RemoteObjectWrapper[boundNames.length];
-
-            outer: for (int ctr = 0; ctr < boundNames.length; ctr++)
-            {
-                int retryCount = 0;
-
-                while (retryCount < 5)
-                {
-                    try
-                    {
-                        remoteObjects[ctr] = getRegistry().lookupWrapper(boundNames[ctr]);
-                        continue outer;
-                    }
-
-                    catch (java.rmi.UnmarshalException e)
-                    {
-                        retryCount += 1;
-                    }
-
-                    catch (Exception e)
-                    {
-                        ExceptionHandler.unexpectedException(e, "lookup", "operation", true);
-                    }
-                }
-
-                remoteObjects[ctr] = new EmptyWrapper(boundNames[ctr]);
-            }
-
+            remoteObjects = getRegistry().lookupWrappers(boundNames);
         }
     }