You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by vk...@apache.org on 2018/05/25 21:24:12 UTC

ignite git commit: IGNITE-8547 - Use JVM serialization for enum values with OptimizedMarshaller, avoid deadlock. - Fixes #4063

Repository: ignite
Updated Branches:
  refs/heads/master 7df2872ca -> 5564a14f7


IGNITE-8547 - Use JVM serialization for enum values with OptimizedMarshaller, avoid deadlock. - Fixes #4063

Signed-off-by: Valentin Kulichenko <va...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/5564a14f
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/5564a14f
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/5564a14f

Branch: refs/heads/master
Commit: 5564a14f7cbf30acb64b457d5bc37780490195c2
Parents: 7df2872
Author: Ilya Kasnacheev <il...@gmail.com>
Authored: Fri May 25 14:23:59 2018 -0700
Committer: Valentin Kulichenko <va...@gmail.com>
Committed: Fri May 25 14:23:59 2018 -0700

----------------------------------------------------------------------
 .../ignite/internal/binary/BinaryUtils.java     |  19 +-
 .../binary/builder/BinaryBuilderSerializer.java |   3 +-
 .../optimized/OptimizedObjectOutputStream.java  |   4 +-
 .../ignite/internal/util/IgniteUtils.java       |  15 ++
 .../MarshallerEnumDeadlockMultiJvmTest.java     | 189 +++++++++++++++++++
 .../IgniteMarshallerSelfTestSuite.java          |   2 +
 6 files changed, 213 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
----------------------------------------------------------------------
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
index 1f167f5..082cc20 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
@@ -600,7 +600,7 @@ public class BinaryUtils {
         if (type != null)
             return type;
 
-        if (isEnum(cls))
+        if (U.isEnum(cls))
             return GridBinaryMarshaller.ENUM;
 
         if (cls.isArray())
@@ -1141,7 +1141,7 @@ public class BinaryUtils {
             return BinaryWriteMode.COL;
         else if (isSpecialMap(cls))
             return BinaryWriteMode.MAP;
-        else if (isEnum(cls))
+        else if (U.isEnum(cls))
             return BinaryWriteMode.ENUM;
         else if (cls == BinaryEnumObjectImpl.class)
             return BinaryWriteMode.BINARY_ENUM;
@@ -1175,21 +1175,6 @@ public class BinaryUtils {
     }
 
     /**
-     * Check if class represents a Enum.
-     *
-     * @param cls Class.
-     * @return {@code True} if this is a Enum class.
-     */
-    public static boolean isEnum(Class cls) {
-        if (cls.isEnum())
-            return true;
-
-        Class sCls = cls.getSuperclass();
-
-        return sCls != null && sCls.isEnum();
-    }
-
-    /**
      * @return Value.
      */
     public static byte[] doReadByteArray(BinaryInputStream in) {

http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
----------------------------------------------------------------------
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
index 018444c..42f6873 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
@@ -27,6 +27,7 @@ import org.apache.ignite.internal.binary.BinaryObjectExImpl;
 import org.apache.ignite.internal.binary.BinaryUtils;
 import org.apache.ignite.internal.binary.BinaryWriterExImpl;
 import org.apache.ignite.internal.binary.GridBinaryMarshaller;
+import org.apache.ignite.internal.util.IgniteUtils;
 
 /**
  *
@@ -110,7 +111,7 @@ class BinaryBuilderSerializer {
             return;
         }
 
-        if (BinaryUtils.isEnum(val.getClass())) {
+        if (IgniteUtils.isEnum(val.getClass())) {
             String clsName = ((Enum)val).getDeclaringClass().getName();
 
             int typeId = writer.context().typeId(clsName);

http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java
----------------------------------------------------------------------
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java b/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java
index 66da2da..fadbec6 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java
@@ -42,6 +42,7 @@ import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.internal.util.GridHandleTable;
 import org.apache.ignite.internal.util.io.GridDataOutput;
 import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.lang.IgniteBiTuple;
 import org.apache.ignite.marshaller.MarshallerContext;
 
@@ -180,7 +181,8 @@ class OptimizedObjectOutputStream extends ObjectOutputStream {
         if (obj == null)
             writeByte(NULL);
         else {
-            if (obj instanceof Throwable && !(obj instanceof Externalizable)) {
+            if (obj instanceof Throwable && !(obj instanceof Externalizable) || U.isEnum(obj.getClass())) {
+                // Avoid problems with differing Enum objects or Enum implementation class deadlocks.
                 writeByte(JDK);
 
                 try {

http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
----------------------------------------------------------------------
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
index 2defefa..1e34c2d 100755
--- a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
@@ -6157,6 +6157,21 @@ public abstract class IgniteUtils {
     }
 
     /**
+     * Check if given class represents a Enum.
+     *
+     * @param cls Class to check.
+     * @return {@code True} if this is a Enum class.
+     */
+    public static boolean isEnum(Class cls) {
+        if (cls.isEnum())
+            return true;
+
+        Class sCls = cls.getSuperclass();
+
+        return sCls != null && sCls.isEnum();
+    }
+
+    /**
      * Converts {@link InterruptedException} to {@link IgniteCheckedException}.
      *
      * @param mux Mux to wait on.

http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java
----------------------------------------------------------------------
diff --git a/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java b/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java
new file mode 100644
index 0000000..7042c03
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.ignite.marshaller;
+
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import javax.cache.configuration.Factory;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.binary.BinaryMarshaller;
+import org.apache.ignite.internal.marshaller.optimized.OptimizedMarshaller;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.marshaller.jdk.JdkMarshaller;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+
+/**
+ * Contains test of Enum marshalling with various {@link Marshaller}s. See IGNITE-8547 for details.
+ */
+public class MarshallerEnumDeadlockMultiJvmTest extends GridCommonAbstractTest {
+    /** */
+    private Factory<Marshaller> marshFactory;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String instanceName) throws Exception {
+        return super.getConfiguration(instanceName).setMarshaller(marshFactory.create());
+    }
+
+    /** */
+    public void testJdkMarshaller() throws Exception {
+        marshFactory = new JdkMarshallerFactory();
+
+        runRemoteUnmarshal();
+    }
+
+    /** */
+    public void testOptimizedMarshaller() throws Exception {
+        marshFactory = new OptimizedMarshallerFactory();
+
+        runRemoteUnmarshal();
+    }
+
+    /** */
+    public void testBinaryMarshaller() throws Exception {
+        marshFactory = new BinaryMarshallerFactory();
+
+        runRemoteUnmarshal();
+    }
+
+    /** */
+    private void runRemoteUnmarshal() throws Exception {
+        Ignite ignite = startGrid(0);
+
+        byte[] one = ignite.configuration().getMarshaller().marshal(DeclaredBodyEnum.ONE);
+        byte[] two = ignite.configuration().getMarshaller().marshal(DeclaredBodyEnum.TWO);
+
+        startGrid(1);
+
+        ignite.compute(ignite.cluster().forRemotes()).call(new UnmarshalCallable(one, two));
+    }
+
+    /** {@inheritDoc} */
+    @Override protected boolean isMultiJvm() {
+        return true;
+    }
+
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids();
+    }
+
+    /** */
+    private static class OptimizedMarshallerFactory implements Factory<Marshaller> {
+        /** {@inheritDoc} */
+        @Override public Marshaller create() {
+            return new OptimizedMarshaller(false);
+        }
+    }
+
+    /** */
+    private static class BinaryMarshallerFactory implements Factory<Marshaller> {
+        /** {@inheritDoc} */
+        @Override public Marshaller create() {
+            return new BinaryMarshaller();
+        }
+    }
+
+    /** */
+    private static class JdkMarshallerFactory implements Factory<Marshaller> {
+        /** {@inheritDoc} */
+        @Override public Marshaller create() {
+            return new JdkMarshaller();
+        }
+    }
+
+    /**
+     * Attempts to unmarshal both in-built and inner-class enum values at exactly the same time in multiple threads.
+     */
+    private static class UnmarshalCallable implements IgniteCallable<Object> {
+        /** */
+        private final byte[] one;
+        /** */
+        private final byte[] two;
+        /** */
+        @IgniteInstanceResource
+        private Ignite ign;
+
+        /** */
+        public UnmarshalCallable(byte[] one, byte[] two) {
+            this.one = one;
+            this.two = two;
+        }
+
+        /** {@inheritDoc} */
+        @Override public Object call() throws Exception {
+            ExecutorService executor = Executors.newFixedThreadPool(2);
+
+            final CyclicBarrier start = new CyclicBarrier(2);
+
+            for (int i = 0; i < 2; i++) {
+                final int ii = i;
+
+                executor.execute(new Runnable() {
+                    @Override public void run() {
+                        try {
+                            start.await();
+
+                            if (ii == 0)
+                                ign.configuration().getMarshaller().unmarshal(one, null);
+                            else
+                                ign.configuration().getMarshaller().unmarshal(two, null);
+                        }
+                        catch (Exception e) {
+                            e.printStackTrace();
+                        }
+                    }
+                });
+            }
+
+            try {
+                executor.shutdown();
+
+                executor.awaitTermination(5, TimeUnit.SECONDS);
+
+                if (!executor.isTerminated())
+                    throw new IllegalStateException("Failed to wait for completion");
+            }
+            catch (Exception te) {
+                throw new IllegalStateException("Failed to wait for completion", te);
+            }
+
+            return null;
+        }
+    }
+
+    /** */
+    public enum DeclaredBodyEnum {
+        ONE,
+        TWO {
+            /** {@inheritDoc} */
+            @Override public boolean isSupported() {
+                return false;
+            }
+        };
+
+        /**
+         * A bogus method.
+         */
+        public boolean isSupported() {
+            return true;
+        }
+    }
+}

http://git-wip-us.apache.org/repos/asf/ignite/blob/5564a14f/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java
----------------------------------------------------------------------
diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java
index 2d7f64b..0eb50d7 100644
--- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java
+++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java
@@ -31,6 +31,7 @@ import org.apache.ignite.internal.util.GridHandleTableSelfTest;
 import org.apache.ignite.internal.util.io.GridUnsafeDataInputOutputByteOrderSelfTest;
 import org.apache.ignite.internal.util.io.GridUnsafeDataOutputArraySizingSelfTest;
 import org.apache.ignite.marshaller.GridMarshallerMappingConsistencyTest;
+import org.apache.ignite.marshaller.MarshallerEnumDeadlockMultiJvmTest;
 import org.apache.ignite.marshaller.jdk.GridJdkMarshallerSelfTest;
 import org.apache.ignite.testframework.GridTestUtils;
 
@@ -67,6 +68,7 @@ public class IgniteMarshallerSelfTestSuite extends TestSuite {
         GridTestUtils.addTestIfNeeded(suite, GridHandleTableSelfTest.class, ignoredTests);
         GridTestUtils.addTestIfNeeded(suite, OptimizedMarshallerPooledSelfTest.class, ignoredTests);
         GridTestUtils.addTestIfNeeded(suite, GridMarshallerMappingConsistencyTest.class, ignoredTests);
+        GridTestUtils.addTestIfNeeded(suite, MarshallerEnumDeadlockMultiJvmTest.class, ignoredTests);
 
         return suite;
     }