From: Tuukka Lehtonen Date: Thu, 1 Mar 2018 22:35:49 +0000 (+0200) Subject: NPE fix for Acorn cluster stream undo handling X-Git-Tag: v1.43.0~136^2~566 X-Git-Url: https://gerrit.simantics.org/r/gitweb?p=simantics%2Fplatform.git;a=commitdiff_plain;h=33b349001037197a526a49e5820f0317dc74d934 NPE fix for Acorn cluster stream undo handling refs #7792 Change-Id: If6e7e3d89db26bb7c4aeed06507b02ae34554d00 --- diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java deleted file mode 100644 index 472b4d7b7..000000000 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java +++ /dev/null @@ -1,70 +0,0 @@ -package org.simantics.acorn.internal; - - -public class ClusterChange2 { - public static final int VERSION = 2; - public static final byte SET_IMMUTABLE_OPERATION = 1; // - public static final byte UNDO_VALUE_OPERATION = 2; // - private static final int INCREMENT = 1<<10; -// private boolean dirty = false; -// private byte[] bytes; -// private int byteIndex; -// private ClusterUID clusterUID; -// private ClusterImpl cluster; -// ClusterChange2(ClusterUID clusterUID, ClusterImpl cluster) { -// this.clusterUID = clusterUID; -// this.cluster = cluster; -// init(); -// } -// void init() { -//// System.err.println("clusterChange2 dirty " + cluster.clusterId); -// dirty = false; -// bytes = new byte[INCREMENT]; -// byteIndex = 0; -// addInt(0); // Size of byte vector. Set by flush. -// addInt(VERSION); -// byteIndex = clusterUID.toByte(bytes, 8); -// } -// boolean isDirty() { -// return dirty; -// } -// void flush(GraphSession graphSession) { -//// System.err.println("flush2 clusterChange2 " + dirty + this); -// if (!dirty) -// return; -// Bytes.writeLE(bytes, 0, byteIndex - 4); -// byte[] ops = Arrays.copyOf(bytes, byteIndex); -//// System.err.println("flush2 clusterChange2 " + cluster.clusterId + " " + ops.length + " bytes."); -// graphSession.updateCluster(new UpdateClusterFunction(ops)); -// init(); -// } -// void setImmutable(boolean immutable) { -// dirty = true; -// addByte(SET_IMMUTABLE_OPERATION); -// addByte((byte)(immutable ? -1 : 0)); -// } -// void undoValueEx(int resourceIndex) { -// dirty = true; -// addByte(UNDO_VALUE_OPERATION); -// addInt(resourceIndex); -// } -// private final void checkSpace(int len) { -// if (bytes.length - byteIndex > len) -// return; -// bytes = Arrays.copyOf(bytes, bytes.length + len + INCREMENT); -// } -// private final void addByte(byte value) { -// checkSpace(1); -// bytes[byteIndex++] = value; -// } -// private final void addInt(int value) { -// checkSpace(4); -// Bytes.writeLE(bytes, byteIndex, value); -// byteIndex += 4; -// } -//// private void addLong(long value) { -//// checkSpace(8); -//// Bytes.writeLE(bytes, byteIndex, value); -//// byteIndex += 8; -//// } -} diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor.java index d4381aeca..2885338c7 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor.java @@ -76,11 +76,16 @@ public class ClusterUpdateProcessor extends ClusterUpdateProcessorBase { } + @Override + void setImmutable(boolean value) { + cluster.setImmutable(value, support); + } + public ClusterImpl process(ClusterImpl cluster) throws IllegalAcornStateException { this.cluster = cluster; process(); info.finish(); return this.cluster; } - + } diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java deleted file mode 100644 index 61a8a8a9d..000000000 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.simantics.acorn.internal; - -import org.simantics.acorn.cluster.ClusterImpl; -import org.simantics.acorn.exception.IllegalAcornStateException; -import org.simantics.acorn.lru.ClusterUpdateOperation; -import org.simantics.db.impl.ClusterSupport; - -public class ClusterUpdateProcessor2 extends ClusterUpdateProcessorBase2 { - - final ClusterSupport support; - final ClusterUpdateOperation info; - private ClusterImpl cluster; - - public ClusterUpdateProcessor2(ClusterSupport support, byte[] operations, ClusterUpdateOperation info) { - super(operations); - this.support = support; - this.info = info; - } - - public void process(ClusterImpl cluster) throws IllegalAcornStateException { - this.cluster = cluster; - process(); - info.finish(); - } - - @Override - void setImmutable(boolean value) { - cluster.setImmutable(value, support); - } - -} diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase.java index cd8130d9c..507db1721 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase.java @@ -12,9 +12,15 @@ import org.simantics.acorn.internal.ClusterStream.StmEnum; import org.simantics.db.exception.DatabaseException; import org.simantics.db.service.Bytes; import org.simantics.db.service.ClusterUID; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import fi.vtt.simantics.procore.internal.ClusterChange2; abstract public class ClusterUpdateProcessorBase { - + + private static final Logger LOGGER = LoggerFactory.getLogger(ClusterUpdateProcessorBase.class); + public final static boolean DEBUG = false; final protected ClusterManager manager; @@ -24,9 +30,9 @@ abstract public class ClusterUpdateProcessorBase { final private ClusterUID uid; final private int clusterKey; final public int version; - - final Map clusterKeyCache = new HashMap(); - + + final Map clusterKeyCache = new HashMap<>(); + public int getResourceKey(ClusterUID uid, int index) throws IllegalAcornStateException { Integer match = clusterKeyCache.get(uid); if(match != null) return match+index; @@ -40,7 +46,7 @@ abstract public class ClusterUpdateProcessorBase { this.manager = client; this.bytes = operations; this.len = Bytes.readLE4(bytes, 0)+4; // whatta? - version = Bytes.readLE4(bytes, 4); + this.version = Bytes.readLE4(bytes, 4); long cuid1 = Bytes.readLE8(bytes, 8); long cuid2 = Bytes.readLE8(bytes, 16); uid = ClusterUID.make(cuid1, cuid2); @@ -66,24 +72,21 @@ abstract public class ClusterUpdateProcessorBase { try { create(); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("resource create failed", e); } - } - + private void processDelete() { - int ri = Bytes.readLE2(bytes, pos); pos += 2; - + if(DEBUG) System.err.println("DEBUG: Delete " + ri); - + try { delete(ri); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("resource {} value delete failed", ri, e); } - } private void processModify(int op) { @@ -110,7 +113,8 @@ abstract public class ClusterUpdateProcessorBase { try { modify(clusterKey + ri, offset, size, bytes, pos); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("resource value modify(clusterKey: {}, ri: {}, offset: {}, size: {}, pos: {}) failed", + clusterKey, ri, offset, size, pos, e); } pos += size; @@ -134,7 +138,8 @@ abstract public class ClusterUpdateProcessorBase { try { set(clusterKey+r, valueBuffer, length); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("resource value set(clusterKey: {}, r: {}, length: {}) failed", + clusterKey, r, length, e); } } @@ -160,7 +165,8 @@ abstract public class ClusterUpdateProcessorBase { try { set(clusterKey+r, valueBuffer, length); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("resource value setShort(clusterKey: {}, r: {}, length: {}) failed", + clusterKey, r, length, e); } } @@ -288,7 +294,8 @@ abstract public class ClusterUpdateProcessorBase { try { claim(clusterKey+ri, predicateKey, objectKey, puid, ouid); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("statement add(clusterKey: {}, ri: {}, predicateKey: {}, objectKey: {}, puid: {}, ouid: {}) failed", + clusterKey, ri, predicateKey, objectKey, puid.toString(), ouid.toString(), e); } } else { @@ -301,7 +308,8 @@ abstract public class ClusterUpdateProcessorBase { try { deny(clusterKey+ri, predicateKey, objectKey, puid, ouid); } catch (DatabaseException e) { - e.printStackTrace(); + LOGGER.error("statement deny(clusterKey: {}, ri: {}, predicateKey: {}, objectKey: {}, puid: {}, ouid: {}) failed", + clusterKey, ri, predicateKey, objectKey, puid.toString(), ouid.toString(), e); } } @@ -309,6 +317,14 @@ abstract public class ClusterUpdateProcessorBase { } public void process() throws IllegalAcornStateException { + if (version == ClusterChange.VERSION) { + process1(); + } else if (version == ClusterChange2.VERSION) { + process2(); + } + } + + private void process1() throws IllegalAcornStateException { foreignPos = 0; @@ -463,14 +479,50 @@ abstract public class ClusterUpdateProcessorBase { } } - - + + private void process2() throws IllegalAcornStateException { + + while(pos < len) { + + int op = bytes[pos++]&0xff; + + switch(op) { + + case ClusterChange2.SET_IMMUTABLE_OPERATION: + processSetImmutable(op); + break; + case ClusterChange2.UNDO_VALUE_OPERATION: + processUndoValue(op); + break; + case ClusterChange2.SET_DELETED_OPERATION: + // TODO: implement? + break; + default: + throw new IllegalAcornStateException("Can not process operation " + op + " for cluster " + uid); + + } + } + + } + + private void processSetImmutable(int op) { + int value = bytes[pos++]&0xff; + setImmutable(value > 0); + } + + private void processUndoValue(int op) { + Bytes.readLE4(bytes, pos); + pos+=4; + } + abstract void create() throws DatabaseException; abstract void delete(int resourceIndex) throws DatabaseException; abstract void modify(int resourceKey, long offset, int size, byte[] bytes, int pos) throws DatabaseException; abstract void set(int resourceKey, byte[] bytes, int length) throws DatabaseException; - + abstract void claim(int resourceKey, int predicateKey, int objectKey, ClusterUID puid, ClusterUID ouid) throws DatabaseException; abstract void deny(int resourceKey, int predicateKey, int objectKey, ClusterUID puid, ClusterUID ouid) throws DatabaseException; - + + abstract void setImmutable(boolean value); + } diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java deleted file mode 100644 index 502729c0b..000000000 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.simantics.acorn.internal; - -import org.simantics.acorn.exception.IllegalAcornStateException; -import org.simantics.db.service.Bytes; -import org.simantics.db.service.ClusterUID; - -public abstract class ClusterUpdateProcessorBase2 { - - final private byte[] bytes; - private int pos = 0; - final private int len; - final private ClusterUID uid; - - public ClusterUpdateProcessorBase2(byte[] operations) { - this.bytes = operations; - this.len = Bytes.readLE4(bytes, 0) + 4; // whatta? - int version = Bytes.readLE4(bytes, 4); - assert(version == ClusterChange2.VERSION); - long cuid1 = Bytes.readLE8(bytes, 8); - long cuid2 = Bytes.readLE8(bytes, 16); - pos = 24; - uid = ClusterUID.make(cuid1, cuid2); - } - - public ClusterUID getClusterUID() { - return uid; - } - - private void processSetImmutable(int op) { - int value = bytes[pos++]&0xff; - setImmutable(value > 0); - } - - private void processUndoValue(int op) { - Bytes.readLE4(bytes, pos); - pos+=4; - } - - public void process() throws IllegalAcornStateException { - - while(pos < len) { - - int op = bytes[pos++]&0xff; - - switch(op) { - - case ClusterChange2.SET_IMMUTABLE_OPERATION: - processSetImmutable(op); - break; - case ClusterChange2.UNDO_VALUE_OPERATION: - processUndoValue(op); - break; - default: - throw new IllegalAcornStateException("Can not process cluster " + uid); - - } - } - } - - abstract void setImmutable(boolean value); - -} diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/UndoClusterUpdateProcessor.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/UndoClusterUpdateProcessor.java index 8b3e4f066..e75e2e0ce 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/UndoClusterUpdateProcessor.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/UndoClusterUpdateProcessor.java @@ -13,17 +13,21 @@ import org.simantics.acorn.lru.ClusterChangeSet.Entry; import org.simantics.acorn.lru.ClusterChangeSet.Type; import org.simantics.db.exception.DatabaseException; import org.simantics.db.service.ClusterUID; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase { - + + private static final Logger LOGGER = LoggerFactory.getLogger(UndoClusterUpdateProcessor.class); + public final static boolean DEBUG = false; final private ClusterChangeSet ccs; - + private int oldValuesIndex = 0; private int statementMaskIndex = 0; - - final public List entries = new ArrayList(); + + final public List entries = new ArrayList<>(); public UndoClusterUpdateProcessor(ClusterManager client, ClusterStreamChunk chunk, ClusterChangeSet ccs) throws DatabaseException { super(client, readOperation(client, chunk, ccs)); @@ -31,27 +35,8 @@ public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase { } private static byte[] readOperation(ClusterManager manager, ClusterStreamChunk chunk, ClusterChangeSet ccs) throws AcornAccessVerificationException, IllegalAcornStateException { - -// ClusterStreamChunk chunk; -// manager.streamLRU.acquireMutex(); -// try { -// chunk = ccs.getChunk(manager); -// } catch (Throwable t) { -// throw new IllegalStateException(t); -// } finally { -// manager.streamLRU.releaseMutex(); -// } -// -// chunk.acquireMutex(); -// try { -// chunk.ve - chunk.makeResident(); - return chunk.getOperation(ccs.chunkOffset); -// } catch (Throwable t) { -// throw new IllegalStateException(t); -// } finally { -// chunk.releaseMutex(); -// } + chunk.makeResident(); + return chunk.getOperation(ccs.chunkOffset); } @Override @@ -110,5 +95,10 @@ public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase { } } - + + @Override + void setImmutable(boolean value) { + LOGGER.error("Attempted to undo `setImmutable({})` cluster operation for cluster {} which is not supported.", value, ccs.cuid); + } + } diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterStreamChunk.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterStreamChunk.java index 12d4d55b8..767746eb0 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterStreamChunk.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterStreamChunk.java @@ -9,7 +9,6 @@ import org.simantics.acorn.ClusterManager; import org.simantics.acorn.Persistable; import org.simantics.acorn.exception.AcornAccessVerificationException; import org.simantics.acorn.exception.IllegalAcornStateException; -import org.simantics.acorn.internal.ClusterChange; import org.simantics.acorn.internal.UndoClusterUpdateProcessor; import org.simantics.compressions.CompressionCodec; import org.simantics.compressions.Compressions; @@ -60,8 +59,6 @@ public class ClusterStreamChunk extends LRUObject im if(op.ccs == null) throw new IllegalAcornStateException("Cluster ChangeSet " + ccsId + " was not found."); UndoClusterUpdateProcessor proc = new UndoClusterUpdateProcessor(clusters, this, op.ccs); - if(proc.version != ClusterChange.VERSION) - return null; // This cluster and CCS can still be under preparation => wait clusters.clusterLRU.ensureUpdates(proc.getClusterUID()); diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterUpdateOperation.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterUpdateOperation.java index c23821b79..254fbf55c 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterUpdateOperation.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterUpdateOperation.java @@ -4,10 +4,7 @@ import org.simantics.acorn.ClusterManager; import org.simantics.acorn.cluster.ClusterImpl; import org.simantics.acorn.exception.AcornAccessVerificationException; import org.simantics.acorn.exception.IllegalAcornStateException; -import org.simantics.acorn.internal.ClusterChange; -import org.simantics.acorn.internal.ClusterChange2; import org.simantics.acorn.internal.ClusterUpdateProcessor; -import org.simantics.acorn.internal.ClusterUpdateProcessor2; import org.simantics.db.service.Bytes; import org.simantics.db.service.ClusterUID; @@ -59,22 +56,11 @@ final public class ClusterUpdateOperation { } public void runWithData(byte[] data) throws IllegalAcornStateException, AcornAccessVerificationException { - try { - int version = Bytes.readLE4(data, 4); - if(version == ClusterChange.VERSION) { - ClusterUpdateProcessor processor = new ClusterUpdateProcessor(manager, manager.support, data, this); - ClusterImpl cluster = info.getForUpdate(); - cluster = processor.process(cluster); - manager.update(uid, cluster); - } else if (version == ClusterChange2.VERSION) { - ClusterUpdateProcessor2 processor = new ClusterUpdateProcessor2(manager.support, data, this); - ClusterImpl cluster = info.getForUpdate(); - processor.process(cluster); - manager.update(uid, cluster); - } else { - throw new IllegalAcornStateException("unsupported clusterChange version " + version); - } + ClusterUpdateProcessor processor = new ClusterUpdateProcessor(manager, manager.support, data, this); + ClusterImpl cluster = info.getForUpdate(); + cluster = processor.process(cluster); + manager.update(uid, cluster); } catch (IllegalAcornStateException | AcornAccessVerificationException e) { throw e; } catch (Throwable t) { diff --git a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterChange2.java b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterChange2.java index 57bbd4404..cebcdcf42 100644 --- a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterChange2.java +++ b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterChange2.java @@ -6,7 +6,7 @@ import org.simantics.db.procore.cluster.ClusterImpl; import org.simantics.db.service.Bytes; import org.simantics.db.service.ClusterUID; -class ClusterChange2 { +public class ClusterChange2 { public static final int VERSION = 2; public static final byte SET_IMMUTABLE_OPERATION = 1; // public static final byte UNDO_VALUE_OPERATION = 2; //