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 <[email protected]> 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
