Handle failure in ffi_closure_alloc#1378
Conversation
|
I had a bit of a look for tests of similar things but didn't see anything obvious, nor can I think of good ways to test this change. Some guidance would be appreciated. |
|
Looks like a fix for #1107. |
|
Change looks good to me. Thank you. I agree testing this will be difficult to impossible. You could startup a virtual machine, lock that down, test to run JNA in that setup and observe what happens. I think that the balance between effort and results would tip in a bad way, so I think review is enough here. Could please also have a look at: Lines 3515 to 3535 in a40db1b Lines 3527 and 3529 look pretty similar, so I think a guard should be placed there also. |
Sure, I think bb75249 does the right thing here. |
|
Eyeballed this and looks good to me. To get this in please squash the changes into a single commit. After merge I'll see which architectures I manage to rebuild. |
`ffi_closure_alloc` may fail and return `NULL` if, for instance, we're running in a locked-down operating system that forbids FFI from allocating executable pages of memory in any of the ways that it tries. Today we pass this `NULL` on to `ffi_prep_closure_loc` which triggers a segmentation fault that takes down the whole JVM. With this change we check for a failure in this call and turn it into an `UnsupportedOperationException` so that the caller can handle it more gracefully. Relates elastic/elasticsearch#73309 Relates elastic/elasticsearch#18272
DaveCTurner
force-pushed
the
2021-08-30-ffi_closure_alloc-returns-NULL
branch
from
5299bd5 to
3f5e937
Compare
|
Sure thing. The squashed commit is 3f5e937. |
|
Thank you. |
4 participants
ffi_closure_allocmay fail and returnNULLif, for instance, we'rerunning in a locked-down operating system that forbids FFI from
allocating executable pages of memory in any of the ways that it tries.
Today we pass this
NULLon toffi_prep_closure_locwhich triggers asegmentation fault that takes down the whole JVM. With this change we
check for a failure in this call and turn it into an
UnsupportedOperationExceptionso that the caller can handle it moregracefully.
Relates elastic/elasticsearch#73309
Relates elastic/elasticsearch#18272
Closes #1107