From d5ca4ed76bc83af27f2ade59ce49e35750aa4177 Mon Sep 17 00:00:00 2001 From: Tuukka Lehtonen Date: Fri, 5 May 2017 00:52:29 +0300 Subject: [PATCH] Fixed bad logical bug from Acorn's MainState.load rollback The major bug was the logical not in MainState.load rollback which caused the database revisioning to be started from 0 when the database was *not* empty. It should have been the other way around. Also cleaned up the database head.state validation code by not using exceptions for flow control in validating head.state files. refs #7124 Change-Id: I7cd57fa73d39a637c71159df63566aed5063fc40 --- .../src/org/simantics/acorn/HeadState.java | 18 +++++++++---- .../src/org/simantics/acorn/MainState.java | 25 +++++++------------ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/HeadState.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/HeadState.java index fd38bc985..a6a1622c8 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/HeadState.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/HeadState.java @@ -16,9 +16,13 @@ import org.simantics.databoard.adapter.AdapterConstructionException; import org.simantics.databoard.binding.mutable.MutableVariant; import org.simantics.databoard.serialization.Serializer; import org.simantics.databoard.util.binary.BinaryMemory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class HeadState { + private static transient final Logger LOGGER = LoggerFactory.getLogger(HeadState.class); + public static final String HEAD_STATE = "head.state"; public static final String SHA_1 = "SHA-1"; @@ -41,7 +45,6 @@ public class HeadState { byte[] bytes = Files.readAllBytes(f); MessageDigest sha1 = MessageDigest.getInstance(SHA_1); int digestLength = sha1.getDigestLength(); - sha1.update(bytes, digestLength, bytes.length - digestLength); byte[] newChecksum = sha1.digest(); if (!Arrays.equals(newChecksum, Arrays.copyOfRange(bytes, 0, digestLength))) { @@ -96,7 +99,7 @@ public class HeadState { } } - public static void validateHeadStateIntegrity(Path headState) throws InvalidHeadStateException, IOException { + public static boolean validateHeadStateIntegrity(Path headState) { try { byte[] bytes = Files.readAllBytes(headState); MessageDigest sha1 = MessageDigest.getInstance(SHA_1); @@ -104,12 +107,17 @@ public class HeadState { sha1.update(bytes, digestLength, bytes.length - digestLength); byte[] newChecksum = sha1.digest(); if (!Arrays.equals(newChecksum, Arrays.copyOfRange(bytes, 0, digestLength))) { - throw new InvalidHeadStateException( - "Checksum " + Arrays.toString(newChecksum) + " does not match excpected " - + Arrays.toString(Arrays.copyOfRange(bytes, 0, digestLength)) + " for " + headState.toAbsolutePath()); + LOGGER.error("Checksum " + Arrays.toString(newChecksum) + " does not match excpected " + Arrays.toString(Arrays.copyOfRange(bytes, 0, digestLength)) + " for " + headState.toAbsolutePath()); + return false; } + return true; + } catch (IOException e) { + LOGGER.error("An I/O error occured while validating integrity of head.state", e); + return false; } catch (NoSuchAlgorithmException e) { + LOGGER.error("SHA-1 digest not found, should not happen", e); throw new Error("SHA-1 digest not found, should not happen", e); } } + } diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/MainState.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/MainState.java index 63fc5d493..ebc4079d5 100644 --- a/bundles/org.simantics.acorn/src/org/simantics/acorn/MainState.java +++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/MainState.java @@ -15,7 +15,6 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.simantics.acorn.exception.InvalidHeadStateException; import org.simantics.databoard.Bindings; import org.simantics.databoard.binding.mutable.MutableVariant; import org.simantics.databoard.util.binary.BinaryMemory; @@ -49,10 +48,10 @@ public class MainState implements Serializable { Bindings.getBindingUnchecked(MainState.class)); int latestRevision = state.headDir - 1; try { - HeadState.validateHeadStateIntegrity(directory.resolve(latestRevision + "/" + HeadState.HEAD_STATE)); - archiveRevisionDirectories(directory, latestRevision, rollbackCallback); - return state; - } catch (InvalidHeadStateException e) { + if (HeadState.validateHeadStateIntegrity(directory.resolve(latestRevision + "/" + HeadState.HEAD_STATE))) { + archiveRevisionDirectories(directory, latestRevision, rollbackCallback); + return state; + } LOGGER.warn("Failed to start database from revision " + latestRevision + " stored in " + mainState + ". " + HeadState.HEAD_STATE + " is invalid."); return rollback(directory, rollbackCallback); } catch (FileNotFoundException e) { @@ -61,7 +60,7 @@ public class MainState implements Serializable { } } catch (FileNotFoundException e) { // The database may also be totally empty at this point - if (!listRevisionDirs(directory, true, MainState::isInteger).isEmpty()) + if (listRevisionDirs(directory, true, MainState::isInteger).isEmpty()) return new MainState(0); LOGGER.warn("Unclean exit detected, " + mainState + " not found. Initiating automatic rollback."); @@ -77,7 +76,7 @@ public class MainState implements Serializable { private static MainState rollback(Path directory, Runnable rollbackCallback) throws IOException { LOGGER.warn("Database rollback initiated for " + directory); rollbackCallback.run(); - Path latest = findNewHeadStateDir(directory, rollbackCallback); + Path latest = findNewHeadStateDir(directory); int latestRevision = latest != null ? safeParseInt(-1, latest) : -1; // +1 because we want to return the next head version to use, // not the latest existing version. @@ -126,16 +125,10 @@ public class MainState implements Serializable { * @return * @throws IOException */ - private static Path findNewHeadStateDir(Path directory, Runnable rollbackCallback) throws IOException { - for (Path last : listRevisionDirs(directory, true, MainState::isInteger)) { - try { - HeadState.validateHeadStateIntegrity(last.resolve(HeadState.HEAD_STATE)); + private static Path findNewHeadStateDir(Path directory) throws IOException { + for (Path last : listRevisionDirs(directory, true, MainState::isInteger)) + if (HeadState.validateHeadStateIntegrity(last.resolve(HeadState.HEAD_STATE))) return last; - } catch (IOException | InvalidHeadStateException e) { - // Cleanup is done in {@link cleanRevisionDirectories} method - rollbackCallback.run(); - } - } return null; } -- 2.43.2