[
https://issues.apache.org/jira/browse/BCEL-69?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Emmanuel Bourg updated BCEL-69:
-------------------------------
Description:
I'm one of the developers working on the Jakarta commons sandbox javaflow
project.
This problem is observed in the trunk (as of 2005/08/09), and it's causing some
test failures in the gump build of the said javaflow project.
Here's what I found while tracing through the code.
When LocalVariableGen is created by using the following constructor:
{code}
> public LocalVariableGen(int index, String name, Type type,
> InstructionHandle start, InstructionHandle end) {
> if((index < 0) || (index > Constants.MAX_SHORT))
> throw new ClassGenException("Invalid index index: " + index);
>
> this.name = name;
> this.type = type;
> this.index = index;
> setStart(start);
> setEnd(end);
> }
{code}
During the setStart method, it registers this newly created LocalVariableGen as
a targeter to the InstructionHandle given by the 'start' parameter.
InstructionHandle.targeters is a HashSet. So adding a LocalVariableGen as a
targter means inserting a LocalVariableGen into a HashSet. This involves in
computing the hashCode.
So from within the setStart method, LocalVariableGen.hashCode() is invoked,
since the end instruction isn't set yet, this returns a hashCode based on just
index and start, ignoring the end field.
Then immediately after the setStart method returns, the above constructor now
sets the end instruction. As a result of this, hash code of the
LocalVariableGen changes. This makes it impossible to update the start of
LocalVariableGen correctly, because LocalVariableGen can no longer remove
itself from the targeter set of the previous 'start' instruction.
In other words, the problem is that the LocalVariableGen class computes its
hash code based on mutable fields, yet at the same time it is used in a
HashSet, where objects are not allowed to change its hash code. To the eyes of
a new comer like me, this seems like a rather fundamental design flaw in the
current BCEL library.
SUGGESTED FIX
=============
implement the hashCode method to return a constant value would fix the problem
at the expense of the performance:
{code}
public int hashCode() {
return 42;
}
{code}
Alternatively, instead of using HashSet, use a custom implementation of a Set
where the identity hash code is used instead of Object.hashCode. Although I
haven't tested this approach, I believe this also fixes the problem.
There may be other ways to fix the problem.
was:
I'm one of the developers working on the Jakarta commons sandbox javaflow
project.
This problem is observed in the trunk (as of 2005/08/09), and it's causing some
test failures in the gump build of the said javaflow project.
Here's what I found while tracing through the code.
When LocalVariableGen is created by using the following constructor:
> public LocalVariableGen(int index, String name, Type type,
> InstructionHandle start, InstructionHandle end) {
> if((index < 0) || (index > Constants.MAX_SHORT))
> throw new ClassGenException("Invalid index index: " + index);
>
> this.name = name;
> this.type = type;
> this.index = index;
> setStart(start);
> setEnd(end);
> }
During the setStart method, it registers this newly created LocalVariableGen as
a targeter to the InstructionHandle given by the 'start' parameter.
InstructionHandle.targeters is a HashSet. So adding a LocalVariableGen as a
targter means inserting a LocalVariableGen into a HashSet. This involves in
computing the hashCode.
So from within the setStart method, LocalVariableGen.hashCode() is invoked,
since the end instruction isn't set yet, this returns a hashCode based on just
index and start, ignoring the end field.
Then immediately after the setStart method returns, the above constructor now
sets the end instruction. As a result of this, hash code of the LocalVariableGen
changes. This makes it impossible to update the start of LocalVariableGen
correctly, because LocalVariableGen can no longer remove itself from the
targeter set of the previous 'start' instruction.
In other words, the problem is that the LocalVariableGen class computes its hash
code based on mutable fields, yet at the same time it is used in a HashSet,
where objects are not allowed to change its hash code. To the eyes of a new
comer like me, this seems like a rather fundamental design flaw in the current
BCEL library.
SUGGESTED FIX
=============
implement the hashCode method to return a constant value would fix the problem
at the expense of the performance:
public int hashCode() {
return 42;
}
Alternatively, instead of using HashSet, use a custom implementation of a Set
where the identity hash code is used instead of Object.hashCode. Although I
haven't tested this approach, I believe this also fixes the problem.
There may be other ways to fix the problem.
Priority: Major
Environment: (was: Operating System: other
Platform: Other)
Affects Version/s: (was: unspecified)
Fix Version/s: 5.2
Priority: (was: P2)
Severity: (was: normal)
> A bug in LocalVariableGen
> -------------------------
>
> Key: BCEL-69
> URL: https://issues.apache.org/jira/browse/BCEL-69
> Project: Commons BCEL
> Issue Type: Bug
> Components: Main
> Reporter: Kohsuke Kawaguchi
> Assignee: Apache Commons Developers
> Fix For: 5.2
>
>
> I'm one of the developers working on the Jakarta commons sandbox javaflow
> project.
> This problem is observed in the trunk (as of 2005/08/09), and it's causing
> some test failures in the gump build of the said javaflow project.
> Here's what I found while tracing through the code.
> When LocalVariableGen is created by using the following constructor:
> {code}
> > public LocalVariableGen(int index, String name, Type type,
> > InstructionHandle start, InstructionHandle end) {
> > if((index < 0) || (index > Constants.MAX_SHORT))
> > throw new ClassGenException("Invalid index index: " + index);
> >
> > this.name = name;
> > this.type = type;
> > this.index = index;
> > setStart(start);
> > setEnd(end);
> > }
> {code}
> During the setStart method, it registers this newly created LocalVariableGen
> as a targeter to the InstructionHandle given by the 'start' parameter.
> InstructionHandle.targeters is a HashSet. So adding a LocalVariableGen as a
> targter means inserting a LocalVariableGen into a HashSet. This involves in
> computing the hashCode.
> So from within the setStart method, LocalVariableGen.hashCode() is invoked,
> since the end instruction isn't set yet, this returns a hashCode based on
> just index and start, ignoring the end field.
> Then immediately after the setStart method returns, the above constructor now
> sets the end instruction. As a result of this, hash code of the
> LocalVariableGen changes. This makes it impossible to update the start of
> LocalVariableGen correctly, because LocalVariableGen can no longer remove
> itself from the targeter set of the previous 'start' instruction.
> In other words, the problem is that the LocalVariableGen class computes its
> hash code based on mutable fields, yet at the same time it is used in a
> HashSet, where objects are not allowed to change its hash code. To the eyes
> of a new comer like me, this seems like a rather fundamental design flaw in
> the current BCEL library.
> SUGGESTED FIX
> =============
> implement the hashCode method to return a constant value would fix the
> problem at the expense of the performance:
> {code}
> public int hashCode() {
> return 42;
> }
> {code}
> Alternatively, instead of using HashSet, use a custom implementation of a Set
> where the identity hash code is used instead of Object.hashCode. Although I
> haven't tested this approach, I believe this also fixes the problem.
> There may be other ways to fix the problem.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
