IGNITE-8926 : Fixed deadlock when binary metadata got updated while h…

…olding topology read lock.
gridgain · Aug 9, 2018 · 0bf4340 · 0bf4340
1 parent 37f86a9
commit 0bf4340
Show file tree

Hide file tree

Showing 17 changed files with 125 additions and 24 deletions.
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/UnregisteredBinaryTypeException.java b/modules/core/src/main/java/org/apache/ignite/internal/UnregisteredBinaryTypeException.java
@@ -0,0 +1,79 @@
+package org.apache.ignite.internal;
+
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Exception thrown during serialization if binary metadata isn't registered and it's registration isn't allowed.
+ */
+public class UnregisteredBinaryTypeException extends IgniteException {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final int typeId;
+
+    /** */
+    private final BinaryMetadata binaryMetadata;
+
+    /**
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(int typeId, BinaryMetadata binaryMetadata) {
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param msg Error message.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(String msg, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(msg);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param cause Non-null throwable cause.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(Throwable cause, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(cause);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param msg Error message.
+     * @param cause Non-null throwable cause.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(String msg, @Nullable Throwable cause, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(msg, cause);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @return Type ID.
+     */
+    public int typeId() {
+        return typeId;
+    }
+
+    /**
+     * @return Binary metadata.
+     */
+    public BinaryMetadata binaryMetadata() {
+        return binaryMetadata;
+    }
+}
diff --git a/...es/core/src/main/java/org/apache/ignite/internal/binary/BinaryCachingMetadataHandler.java b/...es/core/src/main/java/org/apache/ignite/internal/binary/BinaryCachingMetadataHandler.java
@@ -46,7 +46,7 @@ private BinaryCachingMetadataHandler() {
     }
 
     /** {@inheritDoc} */
-    @Override public synchronized void addMeta(int typeId, BinaryType type) throws BinaryObjectException {
+    @Override public synchronized void addMeta(int typeId, BinaryType type, boolean failIfUnregistered) throws BinaryObjectException {
         synchronized (this) {
             BinaryType oldType = metas.put(typeId, type);
 

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java
@@ -773,7 +773,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
                                     BinaryMetadata meta = new BinaryMetadata(typeId, typeName, collector.meta(),
                                         affKeyFieldName, Collections.singleton(newSchema), false, null);
 
-                                    ctx.updateMetadata(typeId, meta);
+                                    ctx.updateMetadata(typeId, meta, writer.failIfUnregistered());
 
                                     schemaReg.addSchema(newSchema.schemaId(), newSchema);
                                 }
@@ -794,7 +794,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
                         BinaryMetadata meta = new BinaryMetadata(typeId, typeName, stableFieldsMeta,
                             affKeyFieldName, Collections.singleton(stableSchema), false, null);
 
-                        ctx.updateMetadata(typeId, meta);
+                        ctx.updateMetadata(typeId, meta, writer.failIfUnregistered());
 
                         schemaReg.addSchema(stableSchema.schemaId(), stableSchema);
 

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java
@@ -653,7 +653,7 @@ else if (!desc.registered()) {
                         schemas, desc0.isEnum(),
                         cls.isEnum() ? enumMap(cls) : null);
 
-                    metaHnd.addMeta(desc0.typeId(), meta.wrap(this));
+                    metaHnd.addMeta(desc0.typeId(), meta.wrap(this), false);
 
                     return desc0;
                 }
@@ -801,7 +801,7 @@ private BinaryClassDescriptor registerUserClassDescriptor(Class<?> cls, boolean
 
         if (!deserialize)
             metaHnd.addMeta(typeId, new BinaryMetadata(typeId, typeName, desc.fieldsMeta(), affFieldName, null,
-                desc.isEnum(), cls.isEnum() ? enumMap(cls) : null).wrap(this));
+                desc.isEnum(), cls.isEnum() ? enumMap(cls) : null).wrap(this), false);
 
         descByCls.put(cls, desc);
 
@@ -1170,7 +1170,7 @@ public void registerUserType(String clsName,
         }
 
         metaHnd.addMeta(id,
-            new BinaryMetadata(id, typeName, fieldsMeta, affKeyFieldName, null, isEnum, enumMap).wrap(this));
+            new BinaryMetadata(id, typeName, fieldsMeta, affKeyFieldName, null, isEnum, enumMap).wrap(this), false);
     }
 
     /**
@@ -1325,10 +1325,11 @@ public BinaryIdentityResolver identity(int typeId) {
     /**
      * @param typeId Type ID.
      * @param meta Meta data.
+     * @param failIfUnregistered Fail if unregistered.
      * @throws BinaryObjectException In case of error.
      */
-    public void updateMetadata(int typeId, BinaryMetadata meta) throws BinaryObjectException {
-        metaHnd.addMeta(typeId, meta.wrap(this));
+    public void updateMetadata(int typeId, BinaryMetadata meta, boolean failIfUnregistered) throws BinaryObjectException {
+        metaHnd.addMeta(typeId, meta.wrap(this), failIfUnregistered);
     }
 
     /**

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java
@@ -30,9 +30,10 @@ public interface BinaryMetadataHandler {
      *
      * @param typeId Type ID.
      * @param meta Metadata.
+     * @param failIfUnregistered Fail if unregistered.
      * @throws BinaryObjectException In case of error.
      */
-    public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException;
+    public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException;
 
     /**
      * Gets meta data for provided type ID.

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryNoopMetadataHandler.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryNoopMetadataHandler.java
@@ -43,7 +43,7 @@ private BinaryNoopMetadataHandler() {
     }
 
     /** {@inheritDoc} */
-    @Override public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException {
+    @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException {
         // No-op.
     }
 

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java
@@ -115,6 +115,13 @@ public BinaryWriterExImpl(BinaryContext ctx, BinaryOutputStream out, BinaryWrite
         start = out.position();
     }
 
+    /**
+     * @return Fail if unregistered flag value.
+     */
+    public boolean failIfUnregistered() {
+        return failIfUnregistered;
+    }
+
     /**
      * @param failIfUnregistered Fail if unregistered.
      */

diff --git a/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java b/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
@@ -126,7 +126,7 @@ public void writeValue(BinaryWriterExImpl writer, Object val, boolean forceCol,
 
             BinaryMetadata meta = new BinaryMetadata(typeId, typeName, null, null, null, true, enumMap);
 
-            writer.context().updateMetadata(typeId, meta);
+            writer.context().updateMetadata(typeId, meta, writer.failIfUnregistered());
 
             // Need register class for marshaller to be able to deserialize enum value.
             writer.context().descriptorForClass(((Enum)val).getDeclaringClass(), false, false);

diff --git a/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java b/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java
@@ -360,7 +360,7 @@ else if (readCache == null) {
                 ctx.registerUserClassName(typeId, typeName);
 
                 ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, fieldsMeta, affFieldName0,
-                    Collections.singleton(curSchema), false, null));
+                    Collections.singleton(curSchema), false, null), writer.failIfUnregistered());
 
                 schemaReg.addSchema(curSchema.schemaId(), curSchema);
             }

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientBinary.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientBinary.java
@@ -160,7 +160,7 @@ class ClientBinary implements IgniteBinary {
 
         int typeId = ctx.typeId(typeName);
 
-        ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, null, null, null, true, vals));
+        ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, null, null, null, true, vals), false);
 
         return ctx.metadata(typeId);
     }

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
@@ -233,7 +233,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
         private final BinaryMetadataHandler cache = BinaryCachingMetadataHandler.create();
 
         /** {@inheritDoc} */
-        @Override public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException {
+        @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException {
             if (cache.metadata(typeId) == null) {
                 try {
                     ch.request(
@@ -246,7 +246,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
                 }
             }
 
-            cache.addMeta(typeId, meta); // merge
+            cache.addMeta(typeId, meta, false); // merge
         }
 
         /** {@inheritDoc} */
@@ -259,7 +259,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
                 if (meta0 != null) {
                     meta = new BinaryTypeImpl(marsh.context(), meta0);
 
-                    cache.addMeta(typeId, meta);
+                    cache.addMeta(typeId, meta, false);
                 }
             }
 

diff --git a/...n/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessor.java b/...n/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessor.java
@@ -49,9 +49,10 @@ public interface CacheObjectBinaryProcessor extends IgniteCacheObjectProcessor {
     /**
      * @param typeId Type ID.
      * @param newMeta New meta data.
+     * @param failIfUnregistered Fail if unregistered.
      * @throws IgniteException In case of error.
      */
-    public void addMeta(int typeId, final BinaryType newMeta) throws IgniteException;
+    public void addMeta(int typeId, final BinaryType newMeta, boolean failIfUnregistered) throws IgniteException;
 
     /**
      * Adds metadata locally without triggering discovery exchange.

diff --git a/...va/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java b/...va/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java
@@ -42,6 +42,7 @@
 import org.apache.ignite.events.Event;
 import org.apache.ignite.internal.GridKernalContext;
 import org.apache.ignite.internal.IgniteNodeAttributes;
+import org.apache.ignite.internal.UnregisteredBinaryTypeException;
 import org.apache.ignite.internal.binary.BinaryContext;
 import org.apache.ignite.internal.binary.BinaryEnumObjectImpl;
 import org.apache.ignite.internal.binary.BinaryFieldMetadata;
@@ -163,7 +164,7 @@ public CacheObjectBinaryProcessorImpl(GridKernalContext ctx) {
             transport = new BinaryMetadataTransport(metadataLocCache, metadataFileStore, ctx, log);
 
             BinaryMetadataHandler metaHnd = new BinaryMetadataHandler() {
-                @Override public void addMeta(int typeId, BinaryType newMeta) throws BinaryObjectException {
+                @Override public void addMeta(int typeId, BinaryType newMeta, boolean failIfUnregistered) throws BinaryObjectException {
                     assert newMeta != null;
                     assert newMeta instanceof BinaryTypeImpl;
 
@@ -182,7 +183,7 @@ public CacheObjectBinaryProcessorImpl(GridKernalContext ctx) {
 
                     BinaryMetadata newMeta0 = ((BinaryTypeImpl)newMeta).metadata();
 
-                    CacheObjectBinaryProcessorImpl.this.addMeta(typeId, newMeta0.wrap(binaryCtx));
+                    CacheObjectBinaryProcessorImpl.this.addMeta(typeId, newMeta0.wrap(binaryCtx), failIfUnregistered);
                 }
 
                 @Override public BinaryType metadata(int typeId) throws BinaryObjectException {
@@ -436,11 +437,11 @@ public GridBinaryMarshaller marshaller() {
         BinaryMetadata meta = new BinaryMetadata(typeId, typeName, fieldTypeIds, affKeyFieldName, null, isEnum,
             enumMap);
 
-        binaryCtx.updateMetadata(typeId, meta);
+        binaryCtx.updateMetadata(typeId, meta, false);
     }
 
     /** {@inheritDoc} */
-    @Override public void addMeta(final int typeId, final BinaryType newMeta) throws BinaryObjectException {
+    @Override public void addMeta(final int typeId, final BinaryType newMeta, boolean failIfUnregistered) throws BinaryObjectException {
         assert newMeta != null;
         assert newMeta instanceof BinaryTypeImpl;
 
@@ -457,6 +458,9 @@ public GridBinaryMarshaller marshaller() {
             if (mergedMeta == oldMeta)
                 return;
 
+            if (failIfUnregistered)
+                throw new UnregisteredBinaryTypeException(typeId, mergedMeta);
+
             MetadataUpdateResult res = transport.requestMetadataUpdate(mergedMeta).get();
 
             assert res != null;

diff --git a/...rg/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java b/...rg/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java
@@ -37,6 +37,7 @@
 import org.apache.ignite.cluster.ClusterNode;
 import org.apache.ignite.internal.IgniteInternalFuture;
 import org.apache.ignite.internal.NodeStoppingException;
+import org.apache.ignite.internal.UnregisteredBinaryTypeException;
 import org.apache.ignite.internal.UnregisteredClassException;
 import org.apache.ignite.internal.cluster.ClusterTopologyCheckedException;
 import org.apache.ignite.internal.mem.IgniteOutOfMemoryException;
@@ -1792,6 +1793,13 @@ private void updateAllAsyncInternal0(
 
                         ((CacheObjectBinaryProcessorImpl)cacheObjProc).binaryContext().descriptorForClass(ex.cls(), false, false);
                     }
+                    catch (UnregisteredBinaryTypeException ex) {
+                        IgniteCacheObjectProcessor cacheObjProc = ctx.cacheObjects();
+
+                        assert cacheObjProc instanceof CacheObjectBinaryProcessorImpl;
+
+                        ((CacheObjectBinaryProcessorImpl)cacheObjProc).binaryContext().updateMetadata(ex.typeId(), ex.binaryMetadata(), false);
+                    }
                 }
             }
             catch (GridCacheEntryRemovedException e) {

diff --git a/...ore/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java b/...ore/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java
@@ -333,7 +333,7 @@ public PlatformContextImpl(GridKernalContext ctx, PlatformCallbackGateway gate,
         BinaryContext binCtx = cacheObjProc.binaryContext();
 
         for (BinaryMetadata meta : metas)
-            binCtx.updateMetadata(meta.typeId(), meta);
+            binCtx.updateMetadata(meta.typeId(), meta, false);
     }
 
     /** {@inheritDoc} */

diff --git a/.../apache/ignite/internal/processors/platform/client/binary/ClientBinaryTypePutRequest.java b/.../apache/ignite/internal/processors/platform/client/binary/ClientBinaryTypePutRequest.java
@@ -49,7 +49,7 @@ public ClientBinaryTypePutRequest(BinaryRawReaderEx reader) {
     @Override public ClientResponse process(ClientConnectionContext ctx) {
         BinaryContext binCtx = ((CacheObjectBinaryProcessorImpl) ctx.kernalContext().cacheObjects()).binaryContext();
 
-        binCtx.updateMetadata(meta.typeId(), meta);
+        binCtx.updateMetadata(meta.typeId(), meta, false);
 
         return super.process(ctx);
     }

diff --git a/modules/core/src/test/java/org/apache/ignite/internal/binary/TestCachingMetadataHandler.java b/modules/core/src/test/java/org/apache/ignite/internal/binary/TestCachingMetadataHandler.java
@@ -30,7 +30,7 @@ public class TestCachingMetadataHandler implements BinaryMetadataHandler {
     private final ConcurrentHashMap<Integer, BinaryType> metas = new ConcurrentHashMap<>();
 
     /** {@inheritDoc} */
-    @Override public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException {
+    @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException {
         BinaryType otherType = metas.put(typeId, meta);
 
         if (otherType != null)