Fixes ByteBuffer support for read-only buffers:

lz4 / lz4-java

LZ4 compression for Java

Apache License 2.0

1.11k stars 252 forks source link

Fixes ByteBuffer support for read-only buffers: #51

Closed blambov closed 10 years ago

blambov commented 10 years ago

throws exception when a read-only buffer is used as destination
throws exception when a read-only heap buffer is attempted to be used as source for JNI methods
Unsafe implementations bypass getArray to access the address field regardless of read-only-ness
adds tests of read-only sources of all types, including memory-mapped buffer
adds test of read-only destinations.

Adds forgotten range checks in JNI ByteBuffer methods. Fixes incorrect change of ByteBuffer state in Safe HC compressor.

blambov commented 10 years ago

Unfortunately there doesn't seem to be a clean way to access read-only heap buffers for JNI. Rather than depend on Unsafe machinery I chose to leave this option unsupported.

If you prefer something else, let me know. I'm also not sure if it makes sense for the Unsafe implementations to support read-only heap buffers if the JNI ones don't; I can remove this part of the patch if you prefer.

jpountz commented 10 years ago

I agree that the JNI impl should avoid relying on unsafe machinery. But it can also be surprising (in the bad way) that some ByteBuffer impls are not supported, so maybe it should just fall back to the safe Java implementation in the read-only heap buffer case?

blambov commented 10 years ago

No problem. Fallback added, using fastestJavaInstance().

jpountz commented 10 years ago

Merged, thanks!