wangyum opened a new pull request, #8908: URL: https://github.com/apache/hadoop/pull/8908
Fixes [HADOOP-20053](https://issues.apache.org/jira/browse/HADOOP-20053). Contains content generated by Cursor. ### Description of PR A failed `NativeIO.POSIX` initialization (e.g. due to a version mismatch between the Hadoop Java code and an older `libhadoop.so` deployed on the nodes) used to run `fd_deinit()` on the error path, tearing down the shared `java.io.FileDescriptor` class cache that `DomainSocket` may have already initialized via `anchorNative()` (a once-per-JVM call, never re-invoked). Any later `DomainSocket.receiveFileDescriptors0()` call then executed `fd_create()` → `NewObject(env, NULL, NULL)` and crashed the JVM with SIGSEGV inside `jni_NewObject` (observed in production Spark executors reading Parquet from HDFS with short-circuit reads enabled; two hs_err files show the identical PC `jni_NewObject+0x107` with NULL jclass/jmethodID registers and `si_addr=0x0`). Whether a given JVM crashed or only warned depended on class-initialization order between `DomainSocket` and the failing NativeIO initializer. Full root-cause analysis with production evidence in the JIRA. Changes: - `fd_init()` (`file_descriptor.c`): resolve all members into locals first and publish the statics atomically, so a failure part-way through cannot leave a half-initialized cache behind; failures are retry-friendly. - `fd_get()` / `fd_create()` (unix and windows): lazily re-initialize the cache and throw `IllegalStateException` instead of calling JNI with NULL class/constructor/field members. The existing `receiveFileDescriptors0` error path propagates the exception (`ExceptionOccurred` → cleanup closes any received fds → rethrow) and the client falls back to a regular block reader instead of dying. - `NativeIO.c`: drop `fd_deinit(env)` from the three init error paths — the FileDescriptor cache is shared with DomainSocket and must not be torn down by unrelated NativeIO init failures. `fd_deinit()` itself is retained (currently no live caller) for symmetry with `fd_init()` and possible future explicit-shutdown use. - `NativeIO.java`: log init failures at WARN instead of DEBUG (the root cause used to be invisible in production logs) through a shared constant message; the constant must stay a constant expression so javac folds it into the inner classes, whose static initializers can run re-entrantly while the outer class is still initializing (this re-entrancy is real: the native `initNative()` entry point can trigger `NativeIO$POSIX.<clinit>`). Known tradeoff (documented in the source and commit message): `fd_init()` is intentionally not thread-safe; a cold-cache race can leak at most one global ref per racing thread and is caught by the full NULL checks in `fd_get()`/`fd_create()`, so the worst outcome is a spurious `IllegalStateException`. `fd_create()` is only reached from short-circuit shm setup, which is near-serialized by the `DfsClientShmManager` loading lock. ### How was this patch tested? No unit test added: reproducing the failure requires a version-skewed `libhadoop.so`, which cannot be constructed in a unit test. Instead: - The failure itself is evidenced by the production hs_err files attached to the JIRA (repeated SIGSEGV at `jni_NewObject+0x107` with NULL class and constructor registers, identical PC across two independent executors). - The post-fix behavior (graceful `IllegalStateException` propagated through the existing `receiveFileDescriptors0` error handling, fds closed on the error path, client falls back to a regular block reader) is verified by inspection of that code path. - The native changes were syntax-checked locally with `-Wall` against JDK JNI headers (zero errors/warnings, unix and windows sections). - End-to-end verification requires deploying a rebuilt `libhadoop.so` to the skewed environment: expected result is a WARN carrying the underlying init exception at the first short-circuit request plus a regular-read fallback, with no hs_err (this is planned for our internal rollout; production evidence "before" is in the JIRA). - The existing `TestNativeIO` / `TestDomainSocket` suites must continue to pass (CI). ### For code changes: - [x] Does the title of this PR start with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')? - [ ] Object storage: Have the integration tests been executed and the endpoint declared according to the connector-specific documentation? *Note: Automated CI testing doesn't cover all cases so manual testing with cloud storage is still required.* — N/A: this change does not touch object storage or any cloud connector. - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? — No new dependencies added. - [x] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files? — No license/notice changes required (existing code only). ### AI Tooling If an AI tool was used: - [x] The PR includes the phrase "Contains content generated by Cursor" where Cursor is the name of the AI tool used. - [x] My use of AI contributions follows the ASF legal policy https://www.apache.org/legal/generative-tooling.html -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
