Skip to content

Commit fd7dcc2

Browse files
committed
[GR-63542] Backport to 24.2.1: Wasm Memory not cleared when WasmInstance out of scope.
PullRequest: graal/20403
2 parents 814d18d + d48b073 commit fd7dcc2

File tree

91 files changed

+1373
-923
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

91 files changed

+1373
-923
lines changed

wasm/src/org.graalvm.wasm.test/src/org/graalvm/wasm/test/WasmFileSuite.java

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import org.graalvm.wasm.WasmFunctionInstance;
8383
import org.graalvm.wasm.WasmInstance;
8484
import org.graalvm.wasm.WasmLanguage;
85+
import org.graalvm.wasm.WasmStore;
8586
import org.graalvm.wasm.memory.WasmMemory;
8687
import org.graalvm.wasm.memory.WasmMemoryLibrary;
8788
import org.graalvm.wasm.test.options.WasmTestOptions;
@@ -164,8 +165,8 @@ private static boolean isPoorShell() {
164165
return inCI() || inWindows();
165166
}
166167

167-
private static Value findMain(WasmContext wasmContext) {
168-
for (final WasmInstance instance : wasmContext.moduleInstances().values()) {
168+
private static Value findMain(WasmStore wasmStore) {
169+
for (final WasmInstance instance : wasmStore.moduleInstances().values()) {
169170
final WasmFunctionInstance function = instance.inferEntryPoint();
170171
if (function != null) {
171172
return Value.asValue(function);
@@ -192,7 +193,8 @@ private void runInContext(WasmCase testCase, Context context, List<Source> sourc
192193
}
193194

194195
final WasmContext wasmContext = WasmContext.get(null);
195-
final Value mainFunction = findMain(wasmContext);
196+
final var contextStore = wasmContext.contextStore();
197+
final Value mainFunction = findMain(contextStore);
196198

197199
resetStatus(System.out, phaseIcon, phaseLabel);
198200

@@ -222,37 +224,40 @@ private void runInContext(WasmCase testCase, Context context, List<Source> sourc
222224
e.setStackTrace(new StackTraceElement[0]);
223225
throw e;
224226
} finally {
225-
// Save context state, and check that it's consistent with the previous one.
226-
if (iterationNeedsStateCheck(i)) {
227-
final ContextState contextState = saveContext(wasmContext);
228-
if (firstIterationContextState == null) {
229-
firstIterationContextState = contextState;
230-
} else {
231-
assertContextEqual(firstIterationContextState, contextState);
227+
// Context may have already been closed, e.g. by __wasi_proc_exit.
228+
if (!wasmContext.environment().getContext().isClosed()) {
229+
// Save context state, and check that it's consistent with the previous one.
230+
if (iterationNeedsStateCheck(i)) {
231+
final ContextState contextState = saveContext(contextStore);
232+
if (firstIterationContextState == null) {
233+
firstIterationContextState = contextState;
234+
} else {
235+
assertContextEqual(firstIterationContextState, contextState);
236+
}
232237
}
233-
}
234238

235-
// Reset context state.
236-
final boolean reinitMemory = requiresZeroMemory || iterationNeedsStateCheck(i + 1);
237-
if (reinitMemory) {
238-
for (int j = 0; j < wasmContext.memories().count(); ++j) {
239-
WasmMemoryLibrary.getUncached().reset(wasmContext.memories().memory(j));
240-
}
241-
for (int j = 0; j < wasmContext.tables().tableCount(); ++j) {
242-
wasmContext.tables().table(j).reset();
239+
// Reset context state.
240+
final boolean reinitMemory = requiresZeroMemory || iterationNeedsStateCheck(i + 1);
241+
if (reinitMemory) {
242+
for (int j = 0; j < contextStore.memories().count(); ++j) {
243+
WasmMemoryLibrary.getUncached().reset(contextStore.memories().memory(j));
244+
}
245+
for (int j = 0; j < contextStore.tables().tableCount(); ++j) {
246+
contextStore.tables().table(j).reset();
247+
}
243248
}
244-
}
245-
List<WasmInstance> instanceList = new ArrayList<>(wasmContext.moduleInstances().values());
246-
instanceList.sort(Comparator.comparingInt(RuntimeState::startFunctionIndex));
247-
for (WasmInstance instance : instanceList) {
248-
if (!instance.isBuiltin()) {
249-
wasmContext.reinitInstance(instance, reinitMemory);
249+
List<WasmInstance> instanceList = new ArrayList<>(contextStore.moduleInstances().values());
250+
instanceList.sort(Comparator.comparingInt(RuntimeState::startFunctionIndex));
251+
for (WasmInstance instance : instanceList) {
252+
if (!instance.isBuiltin()) {
253+
contextStore.reinitInstance(instance, reinitMemory);
254+
}
250255
}
251-
}
252256

253-
// Reset stdin
254-
if (wasmContext.environment().in() instanceof ByteArrayInputStream) {
255-
wasmContext.environment().in().reset();
257+
// Reset stdin
258+
if (wasmContext.environment().in() instanceof ByteArrayInputStream) {
259+
wasmContext.environment().in().reset();
260+
}
256261
}
257262
}
258263
}
@@ -586,10 +591,10 @@ protected String suiteName() {
586591
return getClass().getSimpleName();
587592
}
588593

589-
private static ContextState saveContext(WasmContext context) {
590-
final MemoryRegistry memories = context.memories().duplicate();
591-
final GlobalRegistry globals = context.globals().duplicate();
592-
return new ContextState(memories, globals, context.fdManager().size());
594+
private static ContextState saveContext(WasmStore store) {
595+
final MemoryRegistry memories = store.memories().duplicate();
596+
final GlobalRegistry globals = store.globals().duplicate();
597+
return new ContextState(memories, globals, store.fdManager().size());
593598
}
594599

595600
private static void assertContextEqual(ContextState expectedState, ContextState actualState) {

wasm/src/org.graalvm.wasm.test/src/org/graalvm/wasm/test/WasmJsApiSuite.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,13 @@ public class WasmJsApiSuite {
107107
private static WasmFunctionInstance createWasmFunctionInstance(WasmContext context, byte[] paramTypes, byte[] resultTypes, RootNode functionRootNode) {
108108
WasmModule module = WasmModule.createBuiltin("dummyModule");
109109
module.allocateFunctionType(paramTypes, resultTypes, context.getContextOptions().supportMultiValue());
110-
WasmFunction func = module.declareExportedFunction(0, "dummyFunction");
110+
WasmFunction func = module.declareFunction(0);
111111
func.setTarget(functionRootNode.getCallTarget());
112-
WasmInstance moduleInstance = new WasmInstance(context, module, context.environment().getContext());
112+
WasmInstance moduleInstance = context.contextStore().readInstance(module);
113+
// Perform normal linking steps, incl. assignTypeEquivalenceClasses().
114+
// Functions need to have type equivalence classes assigned for indirect calls.
115+
moduleInstance.store().linker().tryLink(moduleInstance);
116+
assert func.typeEquivalenceClass() >= 0 : "type equivalence class must be assigned";
113117
return new WasmFunctionInstance(moduleInstance, func, functionRootNode.getCallTarget());
114118
}
115119

@@ -902,9 +906,8 @@ public void testTableInstanceOutOfBoundsSet() throws IOException {
902906
final WasmInstance instance = moduleInstantiate(wasm, binaryWithMixedExports, null);
903907
final InteropLibrary lib = InteropLibrary.getUncached();
904908

905-
WasmContext wasmContext = WasmContext.get(null);
906-
final WasmFunctionInstance functionInstance = createWasmFunctionInstance(wasmContext, WasmType.VOID_TYPE_ARRAY, WasmType.I32_TYPE_ARRAY,
907-
new RootNode(wasmContext.language()) {
909+
final WasmFunctionInstance functionInstance = createWasmFunctionInstance(context, WasmType.VOID_TYPE_ARRAY, WasmType.I32_TYPE_ARRAY,
910+
new RootNode(context.language()) {
908911
@Override
909912
public Object execute(VirtualFrame frame) {
910913
return 42;
@@ -2511,11 +2514,11 @@ private static void runMemoryTest(Consumer<WasmContext> testCase) throws IOExcep
25112514
runTest(options -> options.option("wasm.UseUnsafeMemory", "true"), testCase);
25122515
}
25132516

2514-
private static void runTest(Consumer<WasmContext> testCase) throws IOException {
2517+
public static void runTest(Consumer<WasmContext> testCase) throws IOException {
25152518
runTest(null, testCase);
25162519
}
25172520

2518-
private static void runTest(Consumer<Context.Builder> options, Consumer<WasmContext> testCase) throws IOException {
2521+
public static void runTest(Consumer<Context.Builder> options, Consumer<WasmContext> testCase) throws IOException {
25192522
final Context.Builder contextBuilder = Context.newBuilder(WasmLanguage.ID);
25202523
contextBuilder.option("wasm.Builtins", "testutil:testutil");
25212524
if (options != null) {
@@ -2911,7 +2914,7 @@ public void accept(WasmContext context) {
29112914
(byte) 0x61, (byte) 0x6d, (byte) 0x65, (byte) 0x02, (byte) 0x05, (byte) 0x02, (byte) 0x00, (byte) 0x00, (byte) 0x01, (byte) 0x00,
29122915
};
29132916

2914-
private static WasmInstance moduleInstantiate(WebAssembly wasm, byte[] source, Object importObject) {
2917+
public static WasmInstance moduleInstantiate(WebAssembly wasm, byte[] source, Object importObject) {
29152918
final WasmModule module = wasm.moduleDecode(source);
29162919
return wasm.moduleInstantiate(module, importObject);
29172920
}
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package org.graalvm.wasm.test;
42+
43+
import static org.graalvm.wasm.utils.WasmBinaryTools.compileWat;
44+
45+
import java.io.IOException;
46+
import java.lang.ref.WeakReference;
47+
import java.nio.ByteOrder;
48+
import java.util.ArrayList;
49+
50+
import org.graalvm.polyglot.Context;
51+
import org.graalvm.polyglot.Engine;
52+
import org.graalvm.wasm.WasmModule;
53+
import org.graalvm.wasm.WasmTable;
54+
import org.graalvm.wasm.api.Dictionary;
55+
import org.graalvm.wasm.api.WebAssembly;
56+
import org.junit.Assert;
57+
import org.junit.Test;
58+
59+
import com.oracle.truffle.api.interop.InteropException;
60+
import com.oracle.truffle.api.interop.InteropLibrary;
61+
62+
public class WasmMemoryLeakSuite {
63+
64+
@Test
65+
public void testMemoryLeak() throws IOException, InterruptedException {
66+
final byte[] binary = compileWat("leak", """
67+
(module
68+
(memory (export "mem") 128) ;; 8 MiB
69+
(func $fun (export "fun") (result i32) i32.const 42)
70+
(table (export "tab") 1 funcref)
71+
(elem (i32.const 0) $fun)
72+
)
73+
""");
74+
WasmJsApiSuite.runTest(b -> b.engine(Engine.create()), context -> {
75+
InteropLibrary lib = InteropLibrary.getUncached();
76+
WebAssembly wasm = new WebAssembly(context);
77+
WasmModule module = wasm.moduleDecode(binary);
78+
Object importObject = new Dictionary();
79+
try {
80+
var weakStores = new ArrayList<WeakReference<Object>>();
81+
var weakInstances = new ArrayList<WeakReference<Object>>();
82+
var weakMemories = new ArrayList<WeakReference<Object>>();
83+
var weakFunctions = new ArrayList<WeakReference<Object>>();
84+
var weakTables = new ArrayList<WeakReference<Object>>();
85+
var strongInstances = new ArrayList<>();
86+
for (int iter = 0; iter < 16; iter++) {
87+
var instance = wasm.moduleInstantiate(module, importObject);
88+
Object mem = lib.readMember(instance, "mem");
89+
Object fun = lib.readMember(instance, "fun");
90+
Object tab = lib.readMember(instance, "tab");
91+
92+
weakStores.add(new WeakReference<>(instance.store()));
93+
weakInstances.add(new WeakReference<>(instance));
94+
weakMemories.add(new WeakReference<>(mem));
95+
weakFunctions.add(new WeakReference<>(fun));
96+
weakTables.add(new WeakReference<>(tab));
97+
98+
// a single WasmInstance, WasmFunctionInstance, or WasmMemory instance
99+
// should prevent the context from being closed and garbage-collected.
100+
switch (iter % 4) {
101+
case 0 -> strongInstances.add(instance);
102+
case 1 -> strongInstances.add(fun);
103+
case 2 -> strongInstances.add(mem);
104+
case 3 -> strongInstances.add(tab);
105+
}
106+
}
107+
108+
processReferenceQueueAndGC();
109+
110+
for (int i = 0; i < strongInstances.size(); i++) {
111+
Object instance = strongInstances.get(i);
112+
Assert.assertNotNull(instance);
113+
switch (i % 4) {
114+
case 0 -> {
115+
// WasmInstance
116+
Assert.assertTrue(lib.isMemberReadable(instance, "fun"));
117+
}
118+
case 1 -> {
119+
// WasmFunction
120+
Assert.assertTrue(lib.isExecutable(instance));
121+
Assert.assertEquals(42, lib.execute(instance));
122+
}
123+
case 2 -> {
124+
// WasmMemory
125+
Assert.assertEquals(128 * 64 * 1024, lib.getBufferSize(instance));
126+
Assert.assertEquals(0, lib.readBufferInt(instance, ByteOrder.LITTLE_ENDIAN, 0));
127+
}
128+
case 3 -> {
129+
// WasmTable
130+
// No table interop support (GR-57847).
131+
Object fun = WebAssembly.tableRead((WasmTable) instance, 0);
132+
Assert.assertTrue(lib.isExecutable(fun));
133+
Assert.assertEquals(42, lib.execute(fun));
134+
}
135+
}
136+
}
137+
138+
// memories are kept alive either by the WasmMemory object itself or by the context
139+
Assert.assertTrue("WebAssembly memories should not have been freed yet.",
140+
weakMemories.stream().noneMatch(weakRef -> weakRef.refersTo(null)));
141+
// contexts may be freed in the case of only WasmMemory references
142+
143+
strongInstances.clear();
144+
processReferenceQueueAndGC();
145+
146+
Assert.assertTrue("WebAssembly memories should have been freed.",
147+
weakMemories.stream().anyMatch(weakRef -> weakRef.refersTo(null)));
148+
Assert.assertTrue("WebAssembly stores should have been freed.",
149+
weakStores.stream().anyMatch(weakRef -> weakRef.refersTo(null)));
150+
} catch (InteropException e) {
151+
throw new RuntimeException(e);
152+
}
153+
});
154+
}
155+
156+
private static void processReferenceQueueAndGC() {
157+
System.gc();
158+
Context.create().close(); // process reference queue
159+
System.gc();
160+
}
161+
}

0 commit comments

Comments
 (0)