JetBrains-Research / viktor

Efficient f64-only ndarray in Kotlin
MIT License
125 stars 6 forks source link

JNI code relies on not-necessarily-true assumptions #18

Closed dievsky closed 5 years ago

dievsky commented 5 years ago

A typical example of a JNI implementation:

jdouble *src = reinterpret_cast<jdouble *>(
    env->GetPrimitiveArrayCritical(jsrc, NULL));
jdouble *dst = reinterpret_cast<jdouble *>(
    env->GetPrimitiveArrayCritical(jdst, NULL));
boost::simd::transform(src + src_offset, src + src_offset + length,
                       dst + dst_offset, boost::simd::exp);
env->ReleasePrimitiveArrayCritical(jsrc, src, JNI_ABORT);
env->ReleasePrimitiveArrayCritical(jdst, dst, JNI_ABORT);

The problem here is that GetPrimitiveArrayCritical is not guaranteed to return a pointer to the original array, or in fact anything. According to docs, JVM is only highly likely to provide direct heap access, but it can also return a pointer to a copy, or even NULL, if it was unable to allocate memory for the said copy. However, releasing the array in JNI_ABORT mode says that, if we have received a copy previously, there's no need to propagate the changes to the original array. So in the (unlikely but not impossible) event that JVM decided to copy, the changes will be lost. And in the out-of-memory case, we'll just get a nice and cozy segmentation fault.

In short, we should provide for the case when we've received a copy from the JVM.

dievsky commented 5 years ago

Complication: with concurrent access, we don't know what to do with the copy. Suppose we have a 2E6 double array. Thread 1 wants to exponentiate elements 0 through 1E6, thread 2 wants to exponentiate elements 1E6 through 2E6. Suppose JVM doesn't feel like blocking the GC and instead passes two separate copies to these threads. Both threads perform their tasks and want to commit the copies back, thus entering a race condition: one thread will overwrite the result of the other. Solution: we must refuse to do anything with a copy. If JVM passes us a copy of the destination array, we must immediately release it with JNI_ABORT, signal back to Java that no operation was performed and let fallback implementation deal with it.