Skip to content

Commit aaf9896

Browse files
drrbdrrb
committed
Fixed dropping of GreetingSet
We were incorrectly passing GreetingSet by value, which was causing problems when we tried to free it.
1 parent 82fd3a2 commit aaf9896

5 files changed

Lines changed: 32 additions & 17 deletions

File tree

‎src/main/java/com/github/drrb/javarust/Greeting.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ protected List<String> getFieldOrder() {
4949

5050
@Override
5151
public void close() {
52+
// Turn off "auto-synch". If it is on, JNA will automatically read all fields
53+
// from the struct's memory and update them on the Java object. This synchronization
54+
// occurs after every native method call. If it occurs after we drop the struct, JNA
55+
// will try to read from the freed memory and cause a segmentation fault.
56+
setAutoSynch(false);
5257
// Send the struct back to rust for the memory to be freed
5358
Greetings.INSTANCE.dropGreeting(this);
5459
}

‎src/main/java/com/github/drrb/javarust/GreetingSet.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,12 @@
2626
* A struct that contains an array of structs. This is the Java representation
2727
* of the GreetingSet struct in Rust (see the Rust code).
2828
*/
29-
public class GreetingSet extends Structure {
29+
public class GreetingSet extends Structure implements Closeable {
3030

3131
public static class ByReference extends GreetingSet implements Structure.ByReference {
3232
}
3333

34-
public static class ByValue extends GreetingSet implements Structure.ByValue, Closeable {
35-
36-
@Override
37-
public void close() {
38-
Greetings.INSTANCE.dropGreetingSet(this);
39-
}
40-
34+
public static class ByValue extends GreetingSet implements Structure.ByValue {
4135
}
4236

4337
/**
@@ -48,7 +42,7 @@ public void close() {
4842
*
4943
* NB: We need to explicitly specify that the field is a pointer (i.e. we need
5044
* to use ByReference) because, by default, JNA assumes that struct fields
51-
* are not pointers (i.e. the if you just say "Greeting", JNA assumes
45+
* are not pointers (i.e. if you just say "Greeting", JNA assumes
5246
* "Greeting.ByValue" here).
5347
*/
5448
public Greeting.ByReference greetings;
@@ -88,4 +82,20 @@ public List<Greeting> getGreetings() {
8882
protected List<String> getFieldOrder() {
8983
return Arrays.asList("greetings", "numberOfGreetings");
9084
}
85+
86+
/**
87+
* Send the GreetingSet back to Rust to be dropped.
88+
*
89+
* We do this because JNA doesn't free the memory when the object is garbage collected.
90+
*/
91+
@Override
92+
public void close() {
93+
// Turn off "auto-synch". If it is on, JNA will automatically read all fields
94+
// from the struct's memory and update them on the Java object. This synchronization
95+
// occurs after every native method call. If it occurs after we drop the struct, JNA
96+
// will try to read from the freed memory and cause a segmentation fault.
97+
setAutoSynch(false);
98+
// Send the struct back to rust for the memory to be freed
99+
Greetings.INSTANCE.dropGreetingSet(this);
100+
}
91101
}

‎src/main/java/com/github/drrb/javarust/Greetings.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public interface Greetings extends Library {
7676
* This is the same as returning a {@link GreetingSet.ByReference}. JNA assumes
7777
* it's by reference when it's returned from a native function.
7878
*/
79-
GreetingSet.ByValue renderGreetings();
79+
GreetingSet renderGreetings();
8080

8181
/**
8282
* Passing a callback that will be called from Rust with individual strings
@@ -116,5 +116,5 @@ interface GreetingSetCallback extends Callback {
116116
/**
117117
* Free the memory used by a GreetingSet
118118
*/
119-
void dropGreetingSet(GreetingSet.ByValue greetingSet);
119+
void dropGreetingSet(GreetingSet greetingSet);
120120
}

‎src/main/rust/com/github/drrb/javarust/lib/greetings.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,12 @@ pub extern fn sendGreetings(callback: extern "C" fn(&GreetingSet)) {
158158
/// Example of returning a more complicated struct from Rust
159159
#[no_mangle]
160160
#[allow(non_snake_case)]
161-
pub extern fn renderGreetings() -> GreetingSet {
161+
pub extern fn renderGreetings() -> Box<GreetingSet> {
162162
let greetings = vec![ Greeting::new("Hello!"), Greeting::new("Hello again!") ];
163163

164-
GreetingSet {
164+
Box::new(GreetingSet {
165165
greetings: greetings.into_boxed_slice()
166-
}
166+
})
167167
}
168168

169169
#[no_mangle]
@@ -175,7 +175,7 @@ pub extern fn dropGreeting(_: Box<Greeting>) {
175175

176176
#[no_mangle]
177177
#[allow(non_snake_case)]
178-
pub extern fn dropGreetingSet(_: GreetingSet) {
178+
pub extern fn dropGreetingSet(_: Box<GreetingSet>) {
179179
// Do nothing here. Because we own the GreetingSet here and we're not
180180
// returning it, Rust will assume we don't want it anymore and clean it up.
181181
}

‎src/test/java/com/github/drrb/javarust/GreetingsTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public void apply(GreetingSet.ByReference greetingSet) {
9797
});
9898

9999
List<String> greetingStrings = new LinkedList<>();
100-
for (Greeting greeting: greetings) {
100+
for (Greeting greeting : greetings) {
101101
greetingStrings.add(greeting.getText());
102102
}
103103

@@ -106,7 +106,7 @@ public void apply(GreetingSet.ByReference greetingSet) {
106106

107107
@Test
108108
public void shouldGetAStructFromRustContainingAnArrayOfStructs() {
109-
try (GreetingSet.ByValue result = library.renderGreetings()) {
109+
try (GreetingSet result = library.renderGreetings()) {
110110
List<String> greetings = new LinkedList<>();
111111
for (Greeting greeting : result.getGreetings()) {
112112
greetings.add(greeting.getText());

0 commit comments

Comments
 (0)