Skip to content

Commit 0dcb7a9

Browse files
YeregorixYeregorix
authored
Fix class loading error when a thread is interrupted by making ZipFileSystem's backing channel uninterruptable (#8)
1 parent 9d783ab commit 0dcb7a9

4 files changed

Lines changed: 92 additions & 36 deletions

File tree

‎sm-test/src/test/java/net/minecraftforge/securemodules/test/TestClassLoader.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.nio.file.Paths;
1010
import java.security.cert.Certificate;
1111
import java.util.List;
12+
import java.util.concurrent.CompletableFuture;
1213

1314
import org.junit.jupiter.api.Disabled;
1415
import org.junit.jupiter.api.Test;
@@ -58,7 +59,7 @@ private static ClassLoader setup(SecureJar jar, boolean propagates) throws Excep
5859

5960
private static Class<?> getClass(String name, ClassLoader cl) throws Exception {
6061
var cls = Class.forName(name, false, cl);
61-
assertNotNull(cls, "Failed to find test.Signed");
62+
assertNotNull(cls, "Failed to find " + name);
6263
assertEquals("test", cls.getModule().getName(), "Invalid Module");
6364
return cls;
6465
}
@@ -213,4 +214,25 @@ public static void testPackageInfoBoot() throws Exception {
213214
var info = getClass("test.package-info", cl);
214215
assertEquals(cls.getModule(), info.getModule(), "Mismatched modules");
215216
}
217+
218+
@Test
219+
void testInterruption() throws Exception {
220+
boot("testInterruptionBoot");
221+
}
222+
public static void testInterruptionBoot() throws Exception {
223+
var cl = setup("signed");
224+
225+
CompletableFuture<Void> future = new CompletableFuture<>();
226+
new Thread(() -> {
227+
try {
228+
Thread.currentThread().interrupt();
229+
getClass("test.Signed", cl);
230+
future.complete(null);
231+
} catch (Exception e) {
232+
future.completeExceptionally(e);
233+
}
234+
}).start();
235+
236+
future.join();
237+
}
216238
}

‎src/main/java/cpw/mods/jarhandling/impl/Jar.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
import cpw.mods.jarhandling.JarMetadata;
99
import cpw.mods.jarhandling.SecureJar;
1010
import cpw.mods.util.LambdaExceptionUtils;
11+
import cpw.mods.util.ZipUtils;
1112

1213
import java.io.IOException;
1314
import java.io.InputStream;
1415
import java.io.UncheckedIOException;
1516
import java.lang.module.ModuleDescriptor;
1617
import java.net.URI;
17-
import java.net.URISyntaxException;
1818
import java.nio.file.FileSystem;
1919
import java.nio.file.FileSystemAlreadyExistsException;
2020
import java.nio.file.FileSystems;
@@ -196,6 +196,7 @@ private Path newFileSystem(BiPredicate<String, String> filter, Path[] paths) {
196196
} catch (FileSystemAlreadyExistsException e) {
197197
fs = FileSystems.getFileSystem(uri);
198198
}
199+
ZipUtils.setUninterruptible(ZipUtils.getByteChannel(fs));
199200
} else {
200201
// JarInJar is fucking stupid and breaks horribly if it knows about files directly.
201202
// So because I don't want to go into the rabbit hole that is digging into that
@@ -215,7 +216,7 @@ private Path newFileSystem(BiPredicate<String, String> filter, Path[] paths) {
215216

216217
fs = UFSP.newFileSystem(base, map);
217218
}
218-
} catch (IOException | URISyntaxException e) {
219+
} catch (Throwable e) {
219220
return sneak(e);
220221
}
221222

‎src/main/java/cpw/mods/niofs/union/UnionFileSystem.java‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99
import java.io.FileNotFoundException;
1010
import java.io.IOException;
1111
import java.io.InputStream;
12-
import java.lang.invoke.MethodHandle;
13-
import java.lang.invoke.MethodHandles;
14-
import java.lang.invoke.MethodType;
15-
import java.nio.channels.FileChannel;
1612
import java.nio.channels.SeekableByteChannel;
1713
import java.nio.file.AccessMode;
1814
import java.nio.file.DirectoryStream;
@@ -42,33 +38,11 @@
4238
import java.util.stream.IntStream;
4339
import java.util.stream.StreamSupport;
4440

45-
import net.minecraftforge.unsafe.UnsafeHacks;
41+
import cpw.mods.util.ZipUtils;
4642

4743
public class UnionFileSystem extends FileSystem {
48-
private static final MethodHandle ZIPFS_EXISTS;
49-
private static final MethodHandle ZIPFS_CH;
50-
private static final MethodHandle FCI_UNINTERUPTIBLE;
5144
static final String SEP_STRING = "/";
5245

53-
static {
54-
try {
55-
var hackfield = MethodHandles.Lookup.class.getDeclaredField("IMPL_LOOKUP");
56-
UnsafeHacks.setAccessible(hackfield);
57-
MethodHandles.Lookup hack = (MethodHandles.Lookup) hackfield.get(null);
58-
59-
var clz = Class.forName("jdk.nio.zipfs.ZipPath");
60-
ZIPFS_EXISTS = hack.findSpecial(clz, "exists", MethodType.methodType(boolean.class), clz);
61-
62-
clz = Class.forName("jdk.nio.zipfs.ZipFileSystem");
63-
ZIPFS_CH = hack.findGetter(clz, "ch", SeekableByteChannel.class);
64-
65-
clz = Class.forName("sun.nio.ch.FileChannelImpl");
66-
FCI_UNINTERUPTIBLE = hack.findSpecial(clz, "setUninterruptible", MethodType.methodType(void.class), clz);
67-
} catch (NoSuchFieldException | IllegalAccessException | ClassNotFoundException | NoSuchMethodException e) {
68-
throw new RuntimeException(e);
69-
}
70-
}
71-
7246
public InputStream buildInputStream(final UnionPath path) {
7347
try {
7448
var bytes = Files.readAllBytes(path);
@@ -142,11 +116,9 @@ public UnionFileSystem(final UnionFileSystemProvider provider, final BiPredicate
142116
private static Optional<EmbeddedFileSystemMetadata> openFileSystem(final Path path) {
143117
try {
144118
var zfs = FileSystems.newFileSystem(path);
145-
SeekableByteChannel fci = (SeekableByteChannel) ZIPFS_CH.invoke(zfs);
146-
if (fci instanceof FileChannel) { // we only make file channels uninterruptible because byte channels (JIJ) already are
147-
FCI_UNINTERUPTIBLE.invoke(fci);
148-
}
149-
return Optional.of(new EmbeddedFileSystemMetadata(path, zfs, fci));
119+
SeekableByteChannel ch = ZipUtils.getByteChannel(zfs);
120+
ZipUtils.setUninterruptible(ch);
121+
return Optional.of(new EmbeddedFileSystemMetadata(path, zfs, ch));
150122
} catch (IOException e) {
151123
throw new UncheckedIOException(e);
152124
} catch (Throwable t) {
@@ -245,7 +217,7 @@ private Optional<BasicFileAttributes> getFileAttributes(final Path path) {
245217
private static boolean zipFsExists(UnionFileSystem ufs, Path path) {
246218
try {
247219
if (Optional.ofNullable(ufs.embeddedFileSystems.get(path.getFileSystem())).filter(efs->!efs.fsCh.isOpen()).isPresent()) throw new IllegalStateException("The zip file has closed!");
248-
return (boolean) ZIPFS_EXISTS.invoke(path);
220+
return ZipUtils.exists(path);
249221
} catch (Throwable t) {
250222
throw new IllegalStateException(t);
251223
}

‎src/main/java/cpw/mods/util/ZipUtils.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright (c) Forge Development LLC
3+
* SPDX-License-Identifier: LGPL-2.1-only
4+
*/
5+
6+
package cpw.mods.util;
7+
8+
import net.minecraftforge.unsafe.UnsafeHacks;
9+
10+
import java.lang.invoke.MethodHandle;
11+
import java.lang.invoke.MethodHandles;
12+
import java.lang.invoke.MethodType;
13+
import java.nio.channels.FileChannel;
14+
import java.nio.channels.SeekableByteChannel;
15+
import java.nio.file.FileSystem;
16+
import java.nio.file.Path;
17+
18+
/**
19+
* Hacky utilities to access interals of ZipFileSystem to prevent thread interrrupts for breaking the entire FileSystem for everyone else.
20+
* This is an internal class, not exposed in module-info. If you're using this as a consumer, Don't.
21+
* See: https://github.com/MinecraftForge/SecureModules/pull/8
22+
* The ZipPath.exists invocation is for a performance optimization.
23+
*/
24+
public class ZipUtils {
25+
private static final MethodHandle ZIPFS_EXISTS;
26+
private static final MethodHandle ZIPFS_CH;
27+
private static final MethodHandle FCI_UNINTERUPTIBLE;
28+
29+
static {
30+
try {
31+
var hackfield = MethodHandles.Lookup.class.getDeclaredField("IMPL_LOOKUP");
32+
UnsafeHacks.setAccessible(hackfield);
33+
MethodHandles.Lookup hack = (MethodHandles.Lookup) hackfield.get(null);
34+
35+
var clz = Class.forName("jdk.nio.zipfs.ZipPath");
36+
ZIPFS_EXISTS = hack.findSpecial(clz, "exists", MethodType.methodType(boolean.class), clz);
37+
38+
clz = Class.forName("jdk.nio.zipfs.ZipFileSystem");
39+
ZIPFS_CH = hack.findGetter(clz, "ch", SeekableByteChannel.class);
40+
41+
clz = Class.forName("sun.nio.ch.FileChannelImpl");
42+
FCI_UNINTERUPTIBLE = hack.findSpecial(clz, "setUninterruptible", MethodType.methodType(void.class), clz);
43+
} catch (NoSuchFieldException | IllegalAccessException | ClassNotFoundException | NoSuchMethodException e) {
44+
throw new RuntimeException(e);
45+
}
46+
}
47+
48+
public static SeekableByteChannel getByteChannel(final FileSystem zfs) throws Throwable {
49+
return (SeekableByteChannel) ZIPFS_CH.invoke(zfs);
50+
}
51+
52+
public static void setUninterruptible(final SeekableByteChannel byteChannel) throws Throwable {
53+
if (byteChannel instanceof FileChannel) {
54+
FCI_UNINTERUPTIBLE.invoke(byteChannel);
55+
}
56+
}
57+
58+
public static boolean exists(final Path path) throws Throwable {
59+
return (boolean) ZIPFS_EXISTS.invoke(path);
60+
}
61+
}

0 commit comments

Comments
 (0)