[
https://issues.apache.org/jira/browse/BCEL-267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15789243#comment-15789243
]
Stephan Herrmann edited comment on BCEL-267 at 12/31/16 9:37 AM:
-----------------------------------------------------------------
I finally found the time to create a test case that effectively triggers the
problem (see attached
[^BCEL267--Race-conditions-on-static-fields-in-BranchH.patch]).
The core issue is: two threads with no shared data create an instruction list
each, and add lots of instructions to their respective list. After some random
- but not very big - number of iterations, one of the instruction lists will be
corrupt, having a next or prev pointer pointing into the other list, which can
surface with one of 3 possible symptoms:
- NullPointerException in {{InstructionList.getInstructionHandles(..)}}
- instruction list does not contain the expected instructions
- instruction list has corrupt link structure: {{ih.getNext().getPrev() != ih}}
Re-run the test several times to see all three symptoms (it may even randomly
succeed, but that should be rare).
While running other tests I observed a ClassCastException in
org.apache.bcel.PLSETestCase.testB262() even on unmodified sources.
Given that the entire business of the bogusly shared {{ih_list}} and
{{bh_list}} is performance optimization I took a quick look at the output of
{{PerformanceTest}} but difference looked like normal white noise, i.e.: no
indication at a performance regression due to the fix.
was (Author: sherrmann):
I finally found the time to create a test case that effectively triggers the
problem (see attached patch).
The core issue is: two threads with no shared data create an instruction list
each, and add lots of instructions to their respective list. After some random
- but not very big - number of iterations, one of the instruction lists will be
corrupt, having a next or prev pointer pointing into the other list, which can
surface with one of 3 possible symptoms:
- NullPointerException in {{InstructionList.getInstructionHandles(..)}}
- instruction list does not contain the expected instructions
- instruction list has corrupt link structure: {{ih.getNext().getPrev() != ih}}
Re-run the test several times to see all three symptoms (it may even randomly
succeed, but that should be rare).
While running other tests I observed a ClassCastException in
org.apache.bcel.PLSETestCase.testB262() even on unmodified sources.
Given that the entire business of the bogusly shared {{ih_list}} and
{{bh_list}} is performance optimization I took a quick look at the output of
{{PerformanceTest}} but difference looked like normal white noise, i.e.: no
indication at a performance regression due to the fix.
> Race conditions on static fields in BranchHandle and InstructionHandle
> ----------------------------------------------------------------------
>
> Key: BCEL-267
> URL: https://issues.apache.org/jira/browse/BCEL-267
> Project: Commons BCEL
> Issue Type: Bug
> Components: Main
> Affects Versions: 5.2
> Reporter: Stephan Herrmann
> Attachments:
> BCEL267--Race-conditions-on-static-fields-in-BranchH.patch
>
>
> I have observed race conditions causing NullPointerException like this
> {code}java.lang.NullPointerException
> at
> org.apache.bcel.generic.InstructionList.getInstructionHandles(InstructionList.java:1021)
> at
> org.apache.bcel.generic.InstructionList.findHandle(InstructionList.java:141)
> at org.apache.bcel.generic.MethodGen.<init>(MethodGen.java:194)
> {code}
> In the debugger I could verify that concurrent access to the fields
> {{BranchHandle.bh_list}} or {{InstructionHandle.ih_list}} can cause
> corruption of those lists.
> I succeeded to make the exception less frequent by adding synchronized
> blocks, but still the exception could be observed. I concluded that for safe
> protection the fields would need to be made volatile, which in the end might
> actually defeat their original purpose of optimization.
> I have since then run with a patched version of BCEL, where those static
> fields were simply removed and new Handles were created on every request.
> This variant finally was free of the race condition.
> Seeing activity towards a 6.0 release, I'd appreciate if this change could be
> incorporated.
> The original bug tracking my experiments can be found in Eclipse bugzilla:
> https://bugs.eclipse.org/344350
> The modified classes (based on 5.2) can be found here:
> -
> http://git.eclipse.org/c/objectteams/org.eclipse.objectteams.git/tree/plugins/org.eclipse.objectteams.otre/bcelpatchsrc/org/apache/bcel/generic
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
