From 4951a6f75a94e71abe216e816de9886297756ba8 Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 19 Sep 2024 15:14:32 -0700 Subject: [PATCH 1/4] Improve backend error handling - Fix debug logging for #11088--attempt to create an exception that is its own cause fails. - In case the parser is used after closing, throw an `IllegalStateException` instead of UB. --- .../java/org/enso/syntax2/Parser.java | 18 ++++++++++++++---- .../java/org/enso/base/polyglot/EnsoMeta.java | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 2c375ee84082..91731cdde522 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -75,10 +75,10 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... return false; } - private long state; + private long stateUnlessClosed; private Parser(long stateIn) { - state = stateIn; + stateUnlessClosed = stateIn; } private static native long allocState(); @@ -115,7 +115,16 @@ public long isIdentOrOperator(CharSequence input) { return isIdentOrOperator(inputBuf); } + private long getState() { + if (stateUnlessClosed != 0) { + return stateUnlessClosed; + } else { + throw new IllegalStateException("Parser used after close()"); + } + } + public ByteBuffer parseInputLazy(CharSequence input) { + var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); @@ -123,6 +132,7 @@ public ByteBuffer parseInputLazy(CharSequence input) { } public Tree parse(CharSequence input) { + var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); @@ -140,7 +150,7 @@ public static String getWarningMessage(Warning warning) { @Override public void close() { - freeState(state); - state = 0; + freeState(stateUnlessClosed); + stateUnlessClosed = 0; } } diff --git a/std-bits/base/src/main/java/org/enso/base/polyglot/EnsoMeta.java b/std-bits/base/src/main/java/org/enso/base/polyglot/EnsoMeta.java index d37633481736..6749092ae631 100644 --- a/std-bits/base/src/main/java/org/enso/base/polyglot/EnsoMeta.java +++ b/std-bits/base/src/main/java/org/enso/base/polyglot/EnsoMeta.java @@ -18,7 +18,7 @@ public static Value getType(String moduleName, String typeName) { var ex = new NullPointerException( "Cannot get type for " + moduleName + " type: " + typeName + " at " + module); - ex.initCause(ex); + ex.initCause(e); throw ex; } } From c912a55182da62b2172d9378fc8f288e6ae8d20e Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 19 Sep 2024 17:50:03 -0700 Subject: [PATCH 2/4] Detect race conditions in parser usage --- .../java/org/enso/syntax2/Parser.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 91731cdde522..3afb40eab582 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,6 +5,7 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicInteger; public final class Parser implements AutoCloseable { private static void initializeLibraries() { @@ -76,6 +77,7 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... } private long stateUnlessClosed; + private AtomicInteger mutators = new AtomicInteger(0); private Parser(long stateIn) { stateUnlessClosed = stateIn; @@ -124,14 +126,20 @@ private long getState() { } public ByteBuffer parseInputLazy(CharSequence input) { + if (mutators.get() != 0) throw new IllegalStateException("Race condition detected"); + mutators.getAndIncrement(); var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); + var result = parseTreeLazy(state, inputBuf); + mutators.getAndDecrement(); + return result; } public Tree parse(CharSequence input) { + if (mutators.get() != 0) throw new IllegalStateException("Race condition detected"); + mutators.getAndIncrement(); var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); @@ -141,7 +149,9 @@ public Tree parse(CharSequence input) { var metadata = getMetadata(state); serializedTree.order(ByteOrder.LITTLE_ENDIAN); var message = new Message(serializedTree, input, base, metadata); - return Tree.deserialize(message); + var result = Tree.deserialize(message); + mutators.getAndDecrement(); + return result; } public static String getWarningMessage(Warning warning) { From fb9eb1d6330893f59fcd13831381b6141dc16ef2 Mon Sep 17 00:00:00 2001 From: Kaz Date: Mon, 23 Sep 2024 09:08:36 -0700 Subject: [PATCH 3/4] Revert "Detect race conditions in parser usage" This reverts commit c912a55182da62b2172d9378fc8f288e6ae8d20e. --- .../java/org/enso/syntax2/Parser.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 3afb40eab582..91731cdde522 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,7 +5,6 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; -import java.util.concurrent.atomic.AtomicInteger; public final class Parser implements AutoCloseable { private static void initializeLibraries() { @@ -77,7 +76,6 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... } private long stateUnlessClosed; - private AtomicInteger mutators = new AtomicInteger(0); private Parser(long stateIn) { stateUnlessClosed = stateIn; @@ -126,20 +124,14 @@ private long getState() { } public ByteBuffer parseInputLazy(CharSequence input) { - if (mutators.get() != 0) throw new IllegalStateException("Race condition detected"); - mutators.getAndIncrement(); var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - var result = parseTreeLazy(state, inputBuf); - mutators.getAndDecrement(); - return result; + return parseTreeLazy(state, inputBuf); } public Tree parse(CharSequence input) { - if (mutators.get() != 0) throw new IllegalStateException("Race condition detected"); - mutators.getAndIncrement(); var state = getState(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); @@ -149,9 +141,7 @@ public Tree parse(CharSequence input) { var metadata = getMetadata(state); serializedTree.order(ByteOrder.LITTLE_ENDIAN); var message = new Message(serializedTree, input, base, metadata); - var result = Tree.deserialize(message); - mutators.getAndDecrement(); - return result; + return Tree.deserialize(message); } public static String getWarningMessage(Warning warning) { From f3316622f8876288c0b69cf5b707d9f93a84701a Mon Sep 17 00:00:00 2001 From: Kaz Date: Mon, 23 Sep 2024 09:53:51 -0700 Subject: [PATCH 4/4] Report race conditions --- .../java/org/enso/syntax2/Parser.java | 78 ++++++++++++++----- 1 file changed, 59 insertions(+), 19 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 91731cdde522..bb5e1de7876a 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,8 +5,42 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Supplier; public final class Parser implements AutoCloseable { + private final static class OstensiblySuperfluousMutex { + private final Map mutators = new HashMap<>(); + + private R run(Supplier action) { + var thisThread = Thread.currentThread(); + synchronized (mutators) { + mutators.put(thisThread, thisThread.getStackTrace()); + if (mutators.size() != 1) { + System.err.println("THREAD CONFLICT:"); + var index = 0; + for (var entry : mutators.entrySet()) { + System.err.println("Thread " + index + ": " + entry.getKey()); + System.err.println(Arrays.toString(entry.getValue())); + index += 1; + } + } + } + R result; + synchronized (this) { + result = action.get(); + } + synchronized (mutators) { + mutators.remove(thisThread); + } + return result; + } + } + + private final OstensiblySuperfluousMutex mutex = new OstensiblySuperfluousMutex(); + private static void initializeLibraries() { try { System.loadLibrary("enso_parser"); @@ -108,11 +142,13 @@ public static Parser create() { } public long isIdentOrOperator(CharSequence input) { - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); + return mutex.run(() -> { + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); - return isIdentOrOperator(inputBuf); + return isIdentOrOperator(inputBuf); + }); } private long getState() { @@ -124,24 +160,28 @@ private long getState() { } public ByteBuffer parseInputLazy(CharSequence input) { - var state = getState(); - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); + return mutex.run(() -> { + var state = getState(); + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + return parseTreeLazy(state, inputBuf); + }); } public Tree parse(CharSequence input) { - var state = getState(); - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - var serializedTree = parseTree(state, inputBuf); - var base = getLastInputBase(state); - var metadata = getMetadata(state); - serializedTree.order(ByteOrder.LITTLE_ENDIAN); - var message = new Message(serializedTree, input, base, metadata); - return Tree.deserialize(message); + return mutex.run(() -> { + var state = getState(); + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + var serializedTree = parseTree(state, inputBuf); + var base = getLastInputBase(state); + var metadata = getMetadata(state); + serializedTree.order(ByteOrder.LITTLE_ENDIAN); + var message = new Message(serializedTree, input, base, metadata); + return Tree.deserialize(message); + }); } public static String getWarningMessage(Warning warning) {