volatile fields overwritten with 0 when .hashCode() or .equals() is called

[ghazel]

I have found a very strange bug. Using this simple Structure (and no associated C code, because no C calls are made):

public static class Bug extends Structure {
    public volatile long num;
    protected List<String> getFieldOrder() {
        return Arrays.asList(new String[] { "num" } );
    }
};

And this Java code:

Bug b = new Bug();
b.num = 0xABFFEEEE;
b.writeField("num");

Log.d(TAG, "bug pre: " + b);
b.hashCode();
Log.d(TAG, "bug post: " + b);

After b.hashCode() is called, the memory contents are 0:

bug pre: Mylib$Bug(auto-allocated@0xb50016f8 (8 bytes)) {
  long num@0=ffffffffabffeeee
}
memory dump
[eeeeffab]
[ffffffff]
bug post: Mylib$Bug(auto-allocated@0xb50016f8 (8 bytes)) {
  long num@0=ffffffffabffeeee
}
memory dump
[00000000]
[00000000]

Notice the last memory dump, all the bytes are zero.

The CPU abi here is armeabi-v7a (Android) and the JNA is built from the repo at 9f094c2. Does not happen if num is not volatile.

[ghazel]

The bug seems to come from these lines:

https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1484
https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1486
https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1497

They clear the struct and attempt to re-write the fields, but write() doesn't write volatile fields (https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L715)

(as an aside, that's not exactly how I would expect volatile to work -- I would expect it to always write through to the struct (which it does not without writeField, and always read directly from struct memory (which it does today)).

[twall]

By design, fields marked “volatile” are not written by default (see javadoc in Structure.java:71) and you must use “writeField()” to write the Java side to native memory. JNA uses this as a way of tagging areas of memory that may be updated on the native side independently of the Java code. It provides more fine-grained control than “setAutoWrite(false)”.

Are you saying that the native memory is overwritten in cases other than calling “writeField()”?

On Jul 29, 2015, at 5:28 AM, Greg Hazel notifications@github.com wrote:

The bug seems to come from these lines:

https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1484
https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1486
https://github.com/twall/jna/blob/50fe79d660e1991adcf31fffd1fd59e5202d62e0/src/com/sun/jna/Structure.java#L1497

They clear the struct and attempt to re-write the fields, but write() doesn't write volatile fields.

—
Reply to this email directly or view it on GitHub.

[ghazel]

Right. Native memory is being zeroed by clear() regardless of field volatility.

.hashCode() and .equals() clear the native memory to 0 then immediately avoid to writing the volatile Java fields back to native memory. The result is always a zeroing of native memory, since there is no moment for user code to call writeField().

[twall]

It’s not as much of an issue if the Structure memory is allocated by JNA. I suppose that memory could be passed off to native for ongoing use, but that’s an uncommon pattern. The intent was to capture the nature of native blocks of memory being updated asynchronously, like in a device driver, where you want the native side to have sole write access to a particular field.

On Jul 29, 2015, at 2:10 PM, Greg Hazel notifications@github.com wrote:

Right. Native memory is being zeroed by clear() regardless of field volatility.

.hashCode() and .equals() clear the native memory to 0 then immediately avoid to writing the volatile Java fields back to native memory. The result is always a zeroing of native memory, since there is no moment for user code to call writeField().

—
Reply to this email directly or view it on GitHub.

[ghazel]

JNA allocated structure is just an example. In my application I get the structure by calling a function like:

Bug Mylib.getSomeStruct();

Using that structure in a hash map, or array, or any comparison zeros it out, as demonstrated, so later when I try use the structure it's zero:

void Mylib.useBugForThings(Bug b);

[ghazel]

Why do equals and hashCode clear the native memory at all?

[twall]

Probably to prevent padding bytes from affecting a byte by byte comparison. It was a long time ago and I don't think I fully defined the intent of those functions. The general idea was to have structs with identical field values compare equals, but in practice I've never actually used that "feature"

Sent from my iPhone

On Aug 2, 2015, at 11:00 PM, Greg Hazel notifications@github.com wrote:

Why do equals and hashCode clear the native memory at all?

—
Reply to this email directly or view it on GitHub.

[ghazel]

That's a pretty deep equals, which I wouldn't expect from memcmp() or in general, coming from C. Trying to write automatic Java-style object equality seems pretty hard -- and it'd probably want to run .equals() on each of the field members, no? In that case you wouldn't need to clear the memory at all.

[twall]

I think I was intending a memcmp, and cleared the memory prior to writing the Java fields to native memory in order to get a “clean” comparison. The intent was not to do object-by-object comparison.

That said, it might be worth it to drop the hashCode/equals with side effects and go for a simpler model of “is this the same struct/memory” rather than “are these structs equivalent”.

On Aug 3, 2015, at 4:52 AM, Greg Hazel notifications@github.com wrote:

That's a pretty deep equals, which I wouldn't expect from memcmp() or in general, coming from C. Trying to write automatic Java-style object equality seems pretty hard -- and it'd probably want to run .equals() on each of the field members, no? In that case you wouldn't need to clear the memory at all.

—
Reply to this email directly or view it on GitHub.

[ghazel]

I expected "is this the same struct/memory". If I did hope for “are these structs equivalent”, I would expect to either suffer the potential byte padding differences, or be required to implement .equals() myself (likely in C to begin with).

[twall]

As a matter of fact, the discussion around the issue here (http://stackoverflow.com/questions/141720/how-do-you-compare-structs-for-equality-in-c) would imply that JNA isn’t really doing anyone much benefit by providing a struct comparator.

On Aug 3, 2015, at 7:32 AM, Timothy Wall twalljava@java.net wrote:

I think I was intending a memcmp, and cleared the memory prior to writing the Java fields to native memory in order to get a “clean” comparison. The intent was not to do object-by-object comparison.

That said, it might be worth it to drop the hashCode/equals with side effects and go for a simpler model of “is this the same struct/memory” rather than “are these structs equivalent”.

On Aug 3, 2015, at 4:52 AM, Greg Hazel notifications@github.com wrote:

That's a pretty deep equals, which I wouldn't expect from memcmp() or in general, coming from C. Trying to write automatic Java-style object equality seems pretty hard -- and it'd probably want to run .equals() on each of the field members, no? In that case you wouldn't need to clear the memory at all.

—
Reply to this email directly or view it on GitHub.