Skip to content

Map KstatCtl.kc_chain as a pointer - #1741

Merged
dbwiddis merged 1 commit into
java-native-access:masterfrom
dbwiddis:fix-kstatctl-mapping
Oct 9, 2026
Merged

dbwiddis merged 1 commit into
java-native-access:masterfrom
dbwiddis:fix-kstatctl-mapping

Conversation

@dbwiddis

@dbwiddis dbwiddis commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

kstat_ctl_t declares kc_chain as a kstat_t pointer, but KstatCtl mapped it as an inline Kstat (ByValue) Structure, so JNA computed the structure as 200 bytes against the native 24 (32 on Solaris 11.4). Because a Structure passed by reference is written before and read after every call, each kstat_chain_update, kstat_lookup, kstat_read, and kstat_close wrote 176 bytes past the end of the block kstat_open() allocated, reverting adjacent heap to its contents at the previous call.

This PR maps kc_chain as a Pointer and adds KstatCtl.chain() to return the chain head as a Kstat, using a new Kstat(Pointer) constructor that next() now shares. Solaris 11.4's trailing kc_private field is private to libkstat and deliberately left unmapped; the structure only wraps the pointer kstat_open() returns, so the three-field mapping is a safe prefix there.

This is technically a breaking change: anyone who has mapped the previous kc_chain value will fail compilation. However, that value has never worked, it's a 64-bit timestamp, not a pointer, so the blast radius of this change is probably near-zero.

Existing native code used just the intended pointer value, ignoring JNA's claim it was a timestamp, so this effectively was a race condition if adjacent memory was used by another process: rare in libc malloc handling, common in umem handling.

Fixes #1740

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory KstatCtl#kc_chain and Kstat#ks_next could be Kstat#ByReference implementations. But that would introduce more churn and would need to be checked what happens for the last element (is it correctly mapped to null?

For the changes entry: It might be worth to mention in the breaking changes section that this member was not sensibly usable in the existing mapping to give readers indication why this was not considered and API breach.

@dbwiddis

dbwiddis commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

In theory KstatCtl#kc_chain and Kstat#ks_next could be Kstat#ByReference implementations.

Yep, I thought of that, but that introduces unnecessary autowrites when used. For this particular structure, it's static, opened once and should never in theory need to be rewritten to.

For the changes entry: It might be worth to mention in the breaking changes section that this member was not sensibly usable in the existing mapping to give readers indication why this was not considered and API breach.

Will do.

Also realized I haven't yet covered those two other mappings I noted, I'll add those as well in this same PR.

@dbwiddis
dbwiddis force-pushed the fix-kstatctl-mapping branch from 4fb8ab0 to 0d1cb14 Compare October 9, 2026 00:30
kstat_ctl_t declares kc_chain as a kstat_t pointer, but KstatCtl mapped it
as an inline Kstat structure, so JNA computed the structure as 200 bytes
against the native 24 (32 on Solaris 11.4). Because a Structure passed by
reference is written before and read after every call, each
kstat_chain_update, kstat_lookup, kstat_read, and kstat_close wrote 176
bytes past the end of the block kstat_open() allocated, reverting
adjacent heap to its contents at the previous call.

Map kc_chain as a Pointer and add KstatCtl.chain() to return the chain
head as a Kstat, using a new Kstat(Pointer) constructor that next() now
shares. Solaris 11.4's trailing kc_private field is private to libkstat
and deliberately left unmapped; the structure only wraps the pointer
kstat_open() returns, so the three-field mapping is a safe prefix there.

Also map Kstat.ks_update and Kstat.ks_snapshot, which C declares as
function pointers, as Pointer instead of int. The
layout is unchanged (int plus alignment padding occupied the same 8 bytes
on LP64, and the two are the same size on ILP32), so Kstat stays at 184
bytes with identical offsets.

Verified on Solaris 11.4 SPARC under libumem with UMEM_DEBUG=default:
size 24, a full walk of the 4675-entry chain, and 1000 update/lookup/read
cycles plus kstat_close with no allocator errors.

Fixes java-native-access#1740
@dbwiddis
dbwiddis force-pushed the fix-kstatctl-mapping branch from 0d1cb14 to d62bc2f Compare October 9, 2026 00:39
@dbwiddis
dbwiddis merged commit a800ca8 into java-native-access:master Oct 9, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect mapping of c.s.j.platform.unix.solaris.LibKstat.KstatCtl

2 participants