ArrayStoreException in ArrayUtils.append()

[Brent Easton]

Joel,

I am going to pass this one back to you, I am not sure how to fix this properly.

The problem is in SubMenu.getKeyCommands() calling ArrayUtils.append(myC, c) to append the existing keycommands (c) to the submenu keycommands (myC).

ArrayUtils.append(x, y) creates a new array of the same type as x and copies the elements of x any y into it.

However, in SubMenu, x (SubMenuKeyCommand) is a subclass of y (KeyCommand), so we get an ArrayStoreExceptiontrying to stuff KeyCommands into a SubMenuKeyCommands array.

The pre-arrayUtils version of SubMenu correctly created the target array as type KeyCommand and so does not fail.

Easy fix is to just restore the old code that calls System.arraycopy directly.

A harder fix (and beyond me) is to modify ArrayUtils.append() to check the types of all the arguments for subclasses and create the array of type of the basest class.

B.

Finally reproduced this one - it is only reproducible in the 3.2 trunk,
not in the 3.1 branch.

I'm working on it now.

Strangely, the two bug reports where created using Vassal version
3.2.0-svn5061. How did an extremely recent 3.2 release get out into the
wild?

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

Finally reproduced this one - it is only reproducible in the 3.2 trunk, not i
n the 3.1 branch.

I'm working on it now.

Strangely, the two bug reports where created using Vassal version 3.2.0-svn50
61. How did an extremely recent 3.2 release get out into the wild?

B.

There was one in my builds directory very briefly.

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[Brent Easton]

Strangely, the two bug reports where created using Vassal version

3.2.0-svn50

61. How did an extremely recent 3.2 release get out into the wild?

B.

There was one in my builds directory very briefly.

As mentioned at SF, my copy (and I'm the one that reported the bug)
came from SVN because I hadn't switched the tree over to the 3.1
branch, which I since have done. I don't know about the other guy
that found it.

I have been tracking SVN for some time in the hope that I would be
able to help more, and I haven't had the self control to not use the
same build in my recreation time (thus the bug report) (that and I
hadn't realized that 3.2 had branched off yet).

Jeff

No problem Jeff,

It's good to have the latest trunk being tested as well. This is one of those bugs that would not show up in many modules, it only occurs on counters with Submenus.

Be careful about building modules with 3.2 though. The Zip file handling is broken at the moment, and as we get more changes in, it will generate modules that will generate bug reports in 3.1.

B.


_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

Be careful about building modules with 3.2 though. The Zip file handling is b
roken at the moment,

No it isn't. That should work now.

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[Brent Easton]

Be careful about building modules with 3.2 though. The Zip file handling

is b

roken at the moment,

No it isn't. That should work now.

When I edit an existing module and go into a BasicPiece, there are no image names in the drop-down list. If I try and add a new image, I get

java.io.FileNotFoundException: images\union_base.png (The system cannot
find the path specified)
at java.io.FileInputStream.open(Native Method)
at java.io.FileInputStream.<init>(Unknown Source)
at VASSAL.tools.io.ZipArchive.add(ZipArchive.java:375)
at VASSAL.tools.io.ZipArchive.add(ZipArchive.java:368)
at VASSAL.tools.ArchiveWriter.addFile(ArchiveWriter.java:175)
at VASSAL.tools.ArchiveWriter.addImage(ArchiveWriter.java:130)
at VASSAL.counters.ImagePicker.pickImage(ImagePicker.java:168)
at VASSAL.counters.ImagePicker.mouseReleased(ImagePicker.java:151)

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

Joel,

I am going to pass this one back to you, I am not sure how to fix this proper
ly.

The problem is in SubMenu.getKeyCommands() calling ArrayUtils.append(myC, c)
to append the existing keycommands (c) to the submenu keycommands (myC).

ArrayUtils.append(x, y) creates a new array of the same type as x and copies
the elements of x any y into it.

However, in SubMenu, x (SubMenuKeyCommand) is a subclass of y (KeyCommand), s
o we get an ArrayStoreExceptiontrying to stuff KeyCommands into a SubMenuKeyC
ommands array.

The pre-arrayUtils version of SubMenu correctly created the target array as t
ype KeyCommand and so does not fail.

Easy fix is to just restore the old code that calls System.arraycopy directly
.

A harder fix (and beyond me) is to modify ArrayUtils.append() to check the ty
pes of all the arguments for subclasses and create the array of type of the b
asest class.

Are you saying that a SubMenuKeycommand[] is being created? Can you show me
where that's coming from?

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[Brent Easton]

Are you saying that a SubMenuKeycommand[] is being created? Can you show me
where that's coming from?

VASSAL.counters.SubMenu.java lines 41, 73 and 74


_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

Are you saying that a SubMenuKeycommand[] is being created? Can you show me
where that's coming from?

VASSAL.counters.SubMenu.java lines 41, 73 and 74

I think we're doing something dangerous by having myGetKeyCommands()
widen the type of the array it returns to KeyCommand[], due to the way
that Java handles arrays. If this were C++ and we had an array of
SubMenuKeyCommand pointers, we we could also treat it as an array of
KeyCommand pointers in any context which called for an array of KeyCommand
pointers because arrays in C++ aren't objects, they're just contiguous
blocks of memory. Java will implicitly downcast a SubMenuKeyCommand[]
to a KeyCommand[] for you, but then you're stuck with the problem that
the array is only a KeyCommand[] by polymorphism, and you can't later
recover any information about the downcast from it. Since the only way to
find out the type of an array is by reflection, if your SubMenuKeyCommand[]
is downcast to a KeyCommand[] and then later you need to copy the thing,
it looks like a SubMenuKeyCommand[] again (due to getComponentType()
returning the actual component type), not a KeyCommand[] like we wanted.

This would not be a problem at all if Java didn't have both arrays as
objects and generic type erasure. (Without type erasure, it would be
possible to say "new T[]", where T is a generic type, and thereby avoid
this while problem.) It's annoying that you can't treat a Foo[] which was
declared as a Foo[] and a Foo[] which was declared as a Bar[] (where Bar is
a subclass of Foo) in the same fashion. I thought that was the whole point
of polymorphism.

There are two ways around this that I can see:

1. myGetKeyCommands() should return a real KeyCommand[], or

2. There should be another ArrayUtils.append() which takes as an argument
the Class which should be the component type of the returned array, so
like this:

public static <T> T[] append(Class<? super T> type, T[] a, T... b);

Do you have a preference for or the other one of these (or both)?

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[tar]

All this with the caveat that I'm not actually doing any of the
programming.

On Feb 11, 2009, at 5:41 AM, Joel Uckelman wrote:

1. myGetKeyCommands() should return a real KeyCommand[], or

This seems like the simplest solution. And it also seems to fit the
name of the method a little better.

The question is whether there is anybody who calls this that uses any
methods that are specific to the SubMenuKeyCommand type, so that they
would need to cast the elements of the array to that type.



_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake Thomas Russ:

1. myGetKeyCommands() should return a real KeyCommand[], or

This seems like the simplest solution. And it also seems to fit the
name of the method a little better.

The question is whether there is anybody who calls this that uses any
methods that are specific to the SubMenuKeyCommand type, so that they
would need to cast the elements of the array to that type.

Now that I look at it, the array returned by SubMenu.myGetKeyCommands() is
private, so no other class should be expecting it to return a KeyCommand[]
which is also a KeyCommandSubMenu[]. It would violate encapsulation and be
type-unsafe if anyone was relying on that behavior.

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[Brent Easton]

1. myGetKeyCommands() should return a real KeyCommand[], or

That would seem to be the way to go.

B.


_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

1. myGetKeyCommands() should return a real KeyCommand[], or

That would seem to be the way to go.

B.

I ended up doing the second thing, as it protects us from custom code which
downcasts arrays---but we might still want to go back and do the first thing
as well. (The changes would need to be made to SubMenu and DynamicProperty.)

There's another version of ArrayUtils.append() now, for use when you want
to explicitly specify the type of the output array. The signature looks
like this:

<T,X extends T,Y extends T> T[] append(Class<T[]> type, X[] a, Y... b);

So, if you try to use this, it won't compile unless the contents of the head
and tail both can be put into an array of the requested type---which I think
should wrap up the problem.

I've committed the fix to trunk@5038. Would you check to see if this works
for you?

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake Joel Uckelman:

I ended up doing the second thing, as it protects us from custom code which
downcasts arrays---but we might still want to go back and do the first thing
as well. (The changes would need to be made to SubMenu and DynamicProperty.)

There's another version of ArrayUtils.append() now, for use when you want
to explicitly specify the type of the output array. The signature looks
like this:

<T,X extends T,Y extends T> T[] append(Class<T[]> type, X[] a, Y... b);

So, if you try to use this, it won't compile unless the contents of the head
and tail both can be put into an array of the requested type---which I think
should wrap up the problem.

I've committed the fix to trunk@5038. Would you check to see if this works
for you?

Brent?

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[Brent Easton]

I've committed the fix to trunk@5038. Would you check to see if this

works

for you?

Brent?

Sorry...Yes, that fixes the problem.

B.


_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)

[uckelman]

Thus spake "Brent Easton":

I've committed the fix to trunk@5038. Would you check to see if this

works

for you?

Brent?

Sorry...Yes, that fixes the problem.

B.

As of trunk@5169, I've also implemented the other solution, whereby
myGetKeyCommands() returns only a KeyCommand[] which is properly a
KeyCommand[], so I've now closed this bug.

--
J.

_______________________________________________
Messages mailing list
Messages@forums.vassalengine.org
http://forums.vassalengine.org/mailman/ ... engine.org

Post generated using Mail2Forum (http://www.mail2forum.com)