You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/02/11 13:56:51 UTC
[commons-bcel] branch BCEL-267 created (now a325c68)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a change to branch BCEL-267
in repository https://gitbox.apache.org/repos/asf/commons-bcel.git.
at a325c68 [BCEL-267] Race conditions on static fields in BranchHandle and InstructionHandle.
This branch includes the following new commits:
new a325c68 [BCEL-267] Race conditions on static fields in BranchHandle and InstructionHandle.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[commons-bcel] 01/01: [BCEL-267] Race conditions on static fields
in BranchHandle and InstructionHandle.
Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch BCEL-267
in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
commit a325c685b34cc8a3189c18ee4f6480f6249bc99d
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Feb 11 08:56:44 2019 -0500
[BCEL-267] Race conditions on static fields in BranchHandle and
InstructionHandle.
---
.../java/org/apache/bcel/generic/BranchHandle.java | 22 +---
.../org/apache/bcel/generic/InstructionHandle.java | 22 +---
src/test/java/org/apache/bcel/HandleTestCase.java | 138 +++++++++++++++++++++
3 files changed, 142 insertions(+), 40 deletions(-)
diff --git a/src/main/java/org/apache/bcel/generic/BranchHandle.java b/src/main/java/org/apache/bcel/generic/BranchHandle.java
index 8c1b478..94627f1 100644
--- a/src/main/java/org/apache/bcel/generic/BranchHandle.java
+++ b/src/main/java/org/apache/bcel/generic/BranchHandle.java
@@ -40,28 +40,10 @@ public final class BranchHandle extends InstructionHandle {
bi = i;
}
- /** Factory methods.
+ /** Factory method.
*/
- private static BranchHandle bh_list = null; // List of reusable handles
-
-
static BranchHandle getBranchHandle( final BranchInstruction i ) {
- if (bh_list == null) {
- return new BranchHandle(i);
- }
- final BranchHandle bh = bh_list;
- bh_list = (BranchHandle) bh.getNext();
- bh.setInstruction(i);
- return bh;
- }
-
-
- /** Handle adds itself to the list of resuable handles.
- */
- @Override
- protected void addHandle() {
- super.setNext(bh_list);
- bh_list = this;
+ return new BranchHandle(i);
}
diff --git a/src/main/java/org/apache/bcel/generic/InstructionHandle.java b/src/main/java/org/apache/bcel/generic/InstructionHandle.java
index d3994ce..f91b025 100644
--- a/src/main/java/org/apache/bcel/generic/InstructionHandle.java
+++ b/src/main/java/org/apache/bcel/generic/InstructionHandle.java
@@ -113,19 +113,10 @@ public class InstructionHandle {
setInstruction(i);
}
- private static InstructionHandle ih_list = null; // List of reusable handles
-
-
/** Factory method.
*/
static InstructionHandle getInstructionHandle( final Instruction i ) {
- if (ih_list == null) {
- return new InstructionHandle(i);
- }
- final InstructionHandle ih = ih_list;
- ih_list = ih.next;
- ih.setInstruction(i);
- return ih;
+ return new InstructionHandle(i);
}
@@ -162,16 +153,8 @@ public class InstructionHandle {
}
- /** Overridden in BranchHandle
- */
- protected void addHandle() {
- next = ih_list;
- ih_list = this;
- }
-
-
/**
- * Delete contents, i.e., remove user access and make handle reusable.
+ * Delete contents, i.e., remove user access.
*/
void dispose() {
next = prev = null;
@@ -180,7 +163,6 @@ public class InstructionHandle {
i_position = -1;
attributes = null;
removeAllTargeters();
- addHandle();
}
diff --git a/src/test/java/org/apache/bcel/HandleTestCase.java b/src/test/java/org/apache/bcel/HandleTestCase.java
new file mode 100644
index 0000000..c964360
--- /dev/null
+++ b/src/test/java/org/apache/bcel/HandleTestCase.java
@@ -0,0 +1,138 @@
+package org.apache.bcel;
+
+import org.apache.bcel.generic.GOTO;
+import org.apache.bcel.generic.ILOAD;
+import org.apache.bcel.generic.InstructionHandle;
+import org.apache.bcel.generic.InstructionList;
+import org.apache.bcel.generic.NOP;
+import org.junit.Test;
+
+import junit.framework.AssertionFailedError;
+
+/**
+ * Test for https://issues.apache.org/jira/browse/BCEL-267 "Race conditions on
+ * static fields in BranchHandle and InstructionHandle".
+ */
+public class HandleTestCase {
+
+ static Throwable exception;
+ static final int MAXI = 100;
+ static final int MAXJ = 1000;
+
+ /**
+ * Asserts that branch handles can be added an instruction list, without
+ * corrupting the list.
+ */
+ static void branchHandles() {
+ for (int i = 0; i < MAXI; i++) {
+ final InstructionList list = new InstructionList();
+ final InstructionHandle start = list.append(new NOP());
+ try {
+ for (int j = 0; j < MAXJ; j++) {
+ list.append(new GOTO(start));
+ }
+ final InstructionHandle[] instructionHandles = list.getInstructionHandles();
+ for (int j = 0; j < instructionHandles.length; j++) {
+ final InstructionHandle handle = instructionHandles[j];
+ if (j > 0) {
+ checkLinkage(handle, j);
+ if (start != ((GOTO) handle.getInstruction()).getTarget()) {
+ final AssertionFailedError error = new AssertionFailedError(
+ "unexpected instruction at index " + j);
+ exception = error;
+ throw error;
+ }
+ }
+ }
+ if (exception != null) {
+ return;
+ }
+ } catch (final NullPointerException e) {
+ System.out.println("NPE at i=" + i);
+ exception = e;
+ throw e;
+ }
+ list.dispose(); // this initializes caching of unused instruction handles
+ }
+ }
+
+ /**
+ * Assert that opposite next/prev pairs always match.
+ */
+ static void checkLinkage(final InstructionHandle ih, final int index) {
+ final InstructionHandle prev = ih.getPrev();
+ final InstructionHandle next = ih.getNext();
+ if ((prev != null && prev.getNext() != ih) || (next != null && next.getPrev() != ih)) {
+ final AssertionFailedError error = new AssertionFailedError("corrupt instruction list at index " + index);
+ exception = error;
+ throw error;
+ }
+ }
+
+ /**
+ * Asserts that instruction handles can be added an instruction list, without
+ * corrupting the list.
+ */
+ static void handles() {
+ for (int i = 0; i < MAXI; i++) {
+ final InstructionList list = new InstructionList();
+ try {
+ for (int j = 0; j < MAXJ; j++) {
+ list.append(new ILOAD(j));
+ }
+ final InstructionHandle[] instructionHandles = list.getInstructionHandles();
+ for (int j = 0; j < instructionHandles.length; j++) {
+ final InstructionHandle handle = instructionHandles[j];
+ checkLinkage(handle, j);
+ if (j != ((ILOAD) handle.getInstruction()).getIndex()) {
+ final AssertionFailedError error = new AssertionFailedError("unexpected instruction at index " + j);
+ exception = error;
+ throw error;
+ }
+ }
+ if (exception != null) {
+ return;
+ }
+ } catch (final NullPointerException e) {
+ System.out.println("NPE at i=" + i);
+ exception = e;
+ throw e;
+ }
+ list.dispose(); // this initializes caching of unused instruction handles
+ }
+ }
+
+ /**
+ * Concurrently run the given runnable in two threads.
+ */
+ private void perform(final Runnable r) throws Throwable {
+ exception = null;
+ final Thread t1 = new Thread(r);
+ final Thread t2 = new Thread(r);
+ t1.start();
+ t2.start();
+ t1.join();
+ t2.join();
+ if (exception != null) {
+ throw exception;
+ }
+ }
+
+ /**
+ * Assert that two independent instruction lists can be modified concurrently.
+ * Here: inserting branch instructions.
+ */
+ @Test
+ public void testBranchHandle() throws Throwable {
+ perform(HandleTestCase::branchHandles);
+ }
+
+ /**
+ * Assert that two independent instruction lists can be modified concurrently.
+ * Here: inserting regular instructions.
+ */
+ @Test
+ public void testInstructionHandle() throws Throwable {
+ perform(HandleTestCase::handles);
+ }
+}
\ No newline at end of file