Skip to content

Commit 2bcdf32

Browse files
dfa1claude
dfa1
and
claude
committed
refactor: guard zero-copy heap segments; model ZDICT cover params as layouts
Zero-copy MemorySegment APIs dereference the segment address in C, so a heap-backed segment crashed with a cryptic FFM linker error. Add an isNative() guard (Zstd.requireNative) at each zero-copy entry — compress /decompress on the contexts and stream, plus decompressedSize — failing fast with a clear message. Replace the hand-coded ZDICT_cover/fastCover param struct offsets in optimize() with named MemoryLayout structs; offsets and allocation size now derive from the layout instead of magic numbers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a110d8b commit 2bcdf32

7 files changed

Lines changed: 114 additions & 5 deletions

File tree

‎pom.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
<maven-gpg-plugin.version>3.2.8</maven-gpg-plugin.version>
5555
<maven-source-plugin.version>3.4.0</maven-source-plugin.version>
5656
<maven-javadoc-plugin.version>3.12.0</maven-javadoc-plugin.version>
57+
<maven-release-plugin.version>3.3.1</maven-release-plugin.version>
5758
</properties>
5859

5960
<dependencyManagement>
@@ -175,6 +176,17 @@
175176
</dependency>
176177
</dependencies>
177178
</plugin>
179+
<plugin>
180+
<groupId>org.apache.maven.plugins</groupId>
181+
<artifactId>maven-release-plugin</artifactId>
182+
<version>${maven-release-plugin.version}</version>
183+
<configuration>
184+
<autoVersionSubmodules>true</autoVersionSubmodules>
185+
<tagNameFormat>v@{project.version}</tagNameFormat>
186+
<pushChanges>false</pushChanges>
187+
<localCheckout>true</localCheckout>
188+
</configuration>
189+
</plugin>
178190
</plugins>
179191
</pluginManagement>
180192
<plugins>

‎zstd/src/main/java/io/github/dfa1/zstd/Zstd.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public static byte[] decompress(byte[] compressed, int maxSize) {
9191
/// @return the decompressed length in bytes
9292
/// @throws ZstdException if the frame is invalid or does not store its size
9393
public static long decompressedSize(MemorySegment frame) {
94+
requireNative(frame, "frame");
9495
long size;
9596
try {
9697
size = (long) Bindings.GET_FRAME_CONTENT_SIZE.invokeExact(frame, frame.byteSize());
@@ -275,6 +276,17 @@ private static String errorName(long code) {
275276
}
276277
}
277278

279+
/// Guards a zero-copy entry point: the segment handed to zstd must be backed
280+
/// by native (off-heap) memory, since its address is dereferenced in C. Fails
281+
/// fast with a clear message instead of the FFM linker's cryptic error.
282+
static MemorySegment requireNative(MemorySegment seg, String name) {
283+
if (!seg.isNative()) {
284+
throw new IllegalArgumentException(
285+
name + " must be a native (off-heap) MemorySegment; got a heap segment");
286+
}
287+
return seg;
288+
}
289+
278290
static MemorySegment copyIn(Arena arena, byte[] src) {
279291
MemorySegment seg = arena.allocate(Math.max(src.length, 1));
280292
MemorySegment.copy(src, 0, seg, JAVA_BYTE, 0, src.length);

‎zstd/src/main/java/io/github/dfa1/zstd/ZstdCompressCtx.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ public byte[] compress(byte[] src, ZstdCompressDict dict) {
162162
/// @return the number of bytes written into `dst`
163163
/// @throws ZstdException if `dst` is too small or compression fails
164164
public long compress(MemorySegment dst, MemorySegment src) {
165+
Zstd.requireNative(dst, "dst");
166+
Zstd.requireNative(src, "src");
165167
return Zstd.call(() -> (long) Bindings.COMPRESS2.invokeExact(
166168
ptr(), dst, dst.byteSize(), src, src.byteSize()));
167169
}
@@ -173,6 +175,8 @@ public long compress(MemorySegment dst, MemorySegment src) {
173175
/// @param dict the pre-digested compression dictionary
174176
/// @return the number of bytes written into `dst`
175177
public long compress(MemorySegment dst, MemorySegment src, ZstdCompressDict dict) {
178+
Zstd.requireNative(dst, "dst");
179+
Zstd.requireNative(src, "src");
176180
MemorySegment cdict = dict.ptr();
177181
return Zstd.call(() -> (long) Bindings.COMPRESS_USING_CDICT.invokeExact(
178182
ptr(), dst, dst.byteSize(), src, src.byteSize(), cdict));

‎zstd/src/main/java/io/github/dfa1/zstd/ZstdCompressStream.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ private static MemorySegment create(int level, ZstdDictionary dictionary) {
8686
/// @return how much was consumed and produced, and the remaining hint
8787
/// @throws ZstdException if compression fails
8888
public ZstdStreamResult compress(MemorySegment dst, MemorySegment src, ZstdEndDirective directive) {
89+
Zstd.requireNative(dst, "dst");
90+
Zstd.requireNative(src, "src");
8991
in.set(src, src.byteSize(), 0);
9092
out.set(dst, dst.byteSize(), 0);
9193
long remaining = Zstd.call(() -> (long) Bindings.COMPRESS_STREAM2.invokeExact(

‎zstd/src/main/java/io/github/dfa1/zstd/ZstdDecompressCtx.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ public byte[] decompress(byte[] compressed, int maxSize, ZstdDecompressDict dict
115115
/// @return the number of bytes written into `dst`
116116
/// @throws ZstdException if `dst` is too small or the frame is invalid
117117
public long decompress(MemorySegment dst, MemorySegment src) {
118+
Zstd.requireNative(dst, "dst");
119+
Zstd.requireNative(src, "src");
118120
return Zstd.call(() -> (long) Bindings.DECOMPRESS_DCTX.invokeExact(
119121
ptr(), dst, dst.byteSize(), src, src.byteSize()));
120122
}
@@ -126,6 +128,8 @@ public long decompress(MemorySegment dst, MemorySegment src) {
126128
/// @param dict the pre-digested decompression dictionary
127129
/// @return the number of bytes written into `dst`
128130
public long decompress(MemorySegment dst, MemorySegment src, ZstdDecompressDict dict) {
131+
Zstd.requireNative(dst, "dst");
132+
Zstd.requireNative(src, "src");
129133
MemorySegment ddict = dict.ptr();
130134
return Zstd.call(() -> (long) Bindings.DECOMPRESS_USING_DDICT.invokeExact(
131135
ptr(), dst, dst.byteSize(), src, src.byteSize(), ddict));

‎zstd/src/main/java/io/github/dfa1/zstd/ZstdDictionary.java‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package io.github.dfa1.zstd;
22

33
import java.lang.foreign.Arena;
4+
import java.lang.foreign.MemoryLayout;
5+
import java.lang.foreign.MemoryLayout.PathElement;
46
import java.lang.foreign.MemorySegment;
57
import java.lang.invoke.MethodHandle;
68
import java.nio.charset.StandardCharsets;
79
import java.util.List;
810

911
import static java.lang.foreign.ValueLayout.JAVA_BYTE;
12+
import static java.lang.foreign.ValueLayout.JAVA_DOUBLE;
1013
import static java.lang.foreign.ValueLayout.JAVA_INT;
1114
import static java.lang.foreign.ValueLayout.JAVA_LONG;
1215

@@ -28,6 +31,39 @@
2831
/// }
2932
public final class ZstdDictionary {
3033

34+
// ZDICT_cover_params_t { unsigned k,d,steps,nbThreads; double splitPoint;
35+
// unsigned shrinkDict, shrinkDictMaxRegression; ZDICT_params_t zParams; }
36+
// ZDICT_params_t { int compressionLevel; unsigned notificationLevel, dictID; }
37+
private static final MemoryLayout COVER_PARAMS = MemoryLayout.structLayout(
38+
JAVA_INT.withName("k"),
39+
JAVA_INT.withName("d"),
40+
JAVA_INT.withName("steps"),
41+
JAVA_INT.withName("nbThreads"),
42+
JAVA_DOUBLE.withName("splitPoint"),
43+
JAVA_INT.withName("shrinkDict"),
44+
JAVA_INT.withName("shrinkDictMaxRegression"),
45+
JAVA_INT.withName("compressionLevel"),
46+
JAVA_INT.withName("notificationLevel"),
47+
JAVA_INT.withName("dictID"),
48+
MemoryLayout.paddingLayout(4)); // trailing pad to the C struct's 8-byte alignment
49+
50+
// ZDICT_fastCover_params_t adds `unsigned f` after d and `unsigned accel`
51+
// after splitPoint; the 4-byte gap before the 8-aligned double is explicit.
52+
private static final MemoryLayout FASTCOVER_PARAMS = MemoryLayout.structLayout(
53+
JAVA_INT.withName("k"),
54+
JAVA_INT.withName("d"),
55+
JAVA_INT.withName("f"),
56+
JAVA_INT.withName("steps"),
57+
JAVA_INT.withName("nbThreads"),
58+
MemoryLayout.paddingLayout(4),
59+
JAVA_DOUBLE.withName("splitPoint"),
60+
JAVA_INT.withName("accel"),
61+
JAVA_INT.withName("shrinkDict"),
62+
JAVA_INT.withName("shrinkDictMaxRegression"),
63+
JAVA_INT.withName("compressionLevel"),
64+
JAVA_INT.withName("notificationLevel"),
65+
JAVA_INT.withName("dictID"));
66+
3167
private final byte[] bytes;
3268

3369
private ZstdDictionary(byte[] bytes) {
@@ -154,11 +190,10 @@ private static ZstdDictionary optimize(List<byte[]> samples, int maxDictBytes,
154190
offset += s.length;
155191
}
156192
// zeroed params (auto-tune k/d/steps); set single-threaded + target level.
157-
// fastCover: nbThreads@16, compressionLevel@44, size 56.
158-
// cover: nbThreads@12, compressionLevel@32, size 48.
159-
MemorySegment params = arena.allocate(fast ? 56 : 48);
160-
params.set(JAVA_INT, fast ? 16 : 12, 1);
161-
params.set(JAVA_INT, fast ? 44 : 32, compressionLevel);
193+
MemoryLayout layout = fast ? FASTCOVER_PARAMS : COVER_PARAMS;
194+
MemorySegment params = arena.allocate(layout);
195+
params.set(JAVA_INT, layout.byteOffset(PathElement.groupElement("nbThreads")), 1);
196+
params.set(JAVA_INT, layout.byteOffset(PathElement.groupElement("compressionLevel")), compressionLevel);
162197
MethodHandle handle = fast ? Bindings.ZDICT_OPTIMIZE_FASTCOVER : Bindings.ZDICT_OPTIMIZE_COVER;
163198
MemorySegment dictBuf = arena.allocate(maxDictBytes);
164199
long produced;

‎zstd/src/test/java/io/github/dfa1/zstd/ZstdSegmentTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import static java.lang.foreign.ValueLayout.JAVA_BYTE;
1313
import static org.assertj.core.api.Assertions.assertThat;
14+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1415

1516
class ZstdSegmentTest {
1617

@@ -97,6 +98,45 @@ void roundTripsWithDigestedDictionary() {
9798
}
9899
}
99100

101+
@Nested
102+
class HeapSegmentGuard {
103+
104+
@Test
105+
void compressRejectsHeapSource() {
106+
// Given a heap-backed segment (not off-heap) handed to the zero-copy API
107+
MemorySegment heap = MemorySegment.ofArray(new byte[64]);
108+
try (Arena arena = Arena.ofConfined();
109+
ZstdCompressCtx cctx = new ZstdCompressCtx()) {
110+
MemorySegment dst = arena.allocate(64);
111+
112+
// Then it fails fast with a clear message instead of a cryptic FFM error
113+
assertThatThrownBy(() -> cctx.compress(dst, heap))
114+
.isInstanceOf(IllegalArgumentException.class)
115+
.hasMessageContaining("native");
116+
}
117+
}
118+
119+
@Test
120+
void decompressRejectsHeapDestination() {
121+
MemorySegment heapDst = MemorySegment.ofArray(new byte[64]);
122+
try (Arena arena = Arena.ofConfined();
123+
ZstdDecompressCtx dctx = new ZstdDecompressCtx()) {
124+
MemorySegment src = arena.allocate(64);
125+
126+
assertThatThrownBy(() -> dctx.decompress(heapDst, src))
127+
.isInstanceOf(IllegalArgumentException.class)
128+
.hasMessageContaining("native");
129+
}
130+
}
131+
132+
@Test
133+
void decompressedSizeRejectsHeapFrame() {
134+
assertThatThrownBy(() -> Zstd.decompressedSize(MemorySegment.ofArray(new byte[8])))
135+
.isInstanceOf(IllegalArgumentException.class)
136+
.hasMessageContaining("native");
137+
}
138+
}
139+
100140
private static MemorySegment segmentOf(Arena arena, byte[] bytes) {
101141
MemorySegment seg = arena.allocate(Math.max(bytes.length, 1));
102142
MemorySegment.copy(bytes, 0, seg, JAVA_BYTE, 0, bytes.length);

0 commit comments

Comments
 (0)