Repository navigation
fix: launcher, socket, process and system failures from the OpenJDK sweep #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
05923cb
17bc982
99d1e55
99d314b
960f258
b65920f
2c4c399
7d9e010
417f8dd
566e2ec
e75af28
c8b80af
99daf6a
ca86a4c
9e9fc6f
3d82dc1
571eab1
3950235
f9d07be
ccaaeb5
c915afc
1ee3f31
d087620
048045d
474a74d
3b3d8da
46331d1
d8f1cb3
377f5ca
401353d
ea21954
a49fc1b
742cd6f
2d3c06b
0169e06
7f06a65
69e83b6
f74a1f4
9d9b64a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Class files on the -Xbootclasspath/a path: directories and jars on the host. | ||
|
|
||
| import * as fs from "node:fs"; | ||
| import * as path from "node:path"; | ||
| import { inflateRawSync } from "node:zlib"; | ||
|
|
||
| type Jar = { buf: Buffer; entries: Map<string, { method: number; size: number; at: number }> }; | ||
|
|
||
| function readJar(file: string): Jar | null { | ||
| let buf: Buffer; | ||
| try { buf = fs.readFileSync(file); } catch { return null; } | ||
| // a jar that cannot be read has no classes, as a malformed one on HotSpot's path | ||
| try { | ||
| let end = buf.length - 22; | ||
|
project516-review-bot[bot] marked this conversation as resolved.
Resolved
|
||
| while (end >= 0 && buf.readUInt32LE(end) !== 0x06054b50) end--; | ||
| if (end < 0) return null; | ||
| const entries = new Map<string, { method: number; size: number; at: number }>(); | ||
| let p = buf.readUInt32LE(end + 16); | ||
| for (let n = buf.readUInt16LE(end + 10); n > 0 && buf.readUInt32LE(p) === 0x02014b50; n--) { | ||
| const nameLen = buf.readUInt16LE(p + 28); | ||
| entries.set(buf.toString("utf8", p + 46, p + 46 + nameLen), { method: buf.readUInt16LE(p + 10), size: buf.readUInt32LE(p + 20), at: buf.readUInt32LE(p + 42) }); | ||
| p += 46 + nameLen + buf.readUInt16LE(p + 30) + buf.readUInt16LE(p + 32); | ||
| } | ||
| return { buf, entries }; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| function fromJar(jar: Jar, name: string): Uint8Array | null { | ||
| const e = jar.entries.get(name); | ||
| if (!e) return null; | ||
| try { | ||
| const start = e.at + 30 + jar.buf.readUInt16LE(e.at + 26) + jar.buf.readUInt16LE(e.at + 28); | ||
| const data = jar.buf.subarray(start, start + e.size); | ||
| return e.method === 0 ? data : e.method === 8 ? inflateRawSync(data) : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| // entries is the colon-separated -Xbootclasspath/a value | ||
| export function bootClassPath(entries: string | undefined): ((name: string) => { bytes: Uint8Array; location: string } | null) | undefined { | ||
| if (!entries) return undefined; | ||
| const list = entries.split(":").filter(Boolean).map((e) => ({ entry: path.resolve(e), jar: undefined as Jar | null | undefined })); | ||
| return (name) => { | ||
| for (const e of list) { | ||
| if (e.jar === undefined) e.jar = fs.statSync(e.entry, { throwIfNoEntry: false })?.isDirectory() ? null : readJar(e.entry); | ||
| if (e.jar) { | ||
| const bytes = fromJar(e.jar, `${name}.class`); | ||
| if (bytes) return { bytes, location: e.entry }; | ||
| } else if (e.jar === null) { | ||
| try { return { bytes: fs.readFileSync(path.join(e.entry, `${name}.class`)), location: e.entry }; } catch { /* not here */ } | ||
| } | ||
| } | ||
| return null; | ||
| }; | ||
| } | ||
|
|
||
| // --patch-module module=path:path, as the jdk.module.patch.N properties hold it | ||
| export function bootPatches(props: Record<string, string> | undefined): ((module: string, name: string) => Uint8Array | null) | undefined { | ||
| const paths = new Map<string, string[]>(); | ||
| for (const [k, v] of Object.entries(props ?? {})) { | ||
| const eq = v.indexOf("="); | ||
| if (/^jdk\.module\.patch\.\d+$/.test(k) && eq > 0) paths.set(v.slice(0, eq), [...(paths.get(v.slice(0, eq)) ?? []), v.slice(eq + 1)]); | ||
| } | ||
| if (paths.size === 0) return undefined; | ||
| const finders = new Map([...paths].map(([module, list]) => [module, bootClassPath(list.join(":"))!])); | ||
| return (module, name) => finders.get(module)?.(name)?.bytes ?? null; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,12 @@ export interface JvmOptions { | |
| assertions?: Assertions; | ||
| // run class files that use this release's preview features | ||
| enablePreview?: boolean; | ||
| // class bytes from --patch-module for a boot loader module, which win over the image's | ||
| bootPatch?: (module: string, name: string) => Uint8Array | null; | ||
| // class bytes on the -Xbootclasspath/a path, and the path entry that holds them | ||
| bootClassPath?: (name: string) => { bytes: Uint8Array; location: string } | null; | ||
| // os.version; the kernel release where there is one | ||
| osVersion?: string; | ||
| // sockets and name lookup; defaults to loopback sockets inside the VM | ||
| net?: NetBackend; | ||
| // what InetAddress.getLocalHost() reports | ||
|
|
@@ -97,16 +103,16 @@ export interface JvmOptions { | |
| const RESUMED = Object.freeze({ kind: "resume" }) as unknown as Request; | ||
|
|
||
| // LauncherHelper launch modes | ||
| export const LM_CLASS = 1, LM_JAR = 2; | ||
| export const LM_CLASS = 1, LM_JAR = 2, LM_MODULE = 3; | ||
|
|
||
| const intrinsicClasses = new Set(Object.keys(intrinsics).map((k) => k.slice(0, k.indexOf(".")))); | ||
|
|
||
| // Rejects run and startLibrary when a snapshot could not be restored. The | ||
| // snapshot is dropped, so a new VM with the same options boots. | ||
| export class SnapshotError extends Error {} | ||
|
|
||
| // properties under which boot reads application files, which a key does not cover | ||
| const BOOT_READS_APP = /^(java\.system\.class\.loader|jdk\.module\.(path|upgrade\.path|patch\.\d+))$/; | ||
| // properties under which the VM reads application files that a snapshot key does not cover | ||
| const BOOT_READS_APP = /^(java\.system\.class\.loader|jdk\.boot\.class\.path\.append|jdk\.module\.(path|upgrade\.path|patch\.\d+))$/; | ||
|
|
||
| const YIELD = (() => { | ||
| if (typeof setImmediate === "function") return (fn: () => void) => setImmediate(fn); | ||
|
|
@@ -131,6 +137,10 @@ export class JVM { | |
| private readonly image: JImage | null; | ||
| // package -> module, for boot classes | ||
| readonly bootPackages = new Map<string, string>(); | ||
| // packages of modules defined at run time for the boot loader, such as a Proxy's dynamic module | ||
| readonly bootModulePackages = new Map<string, JObject>(); | ||
| // packages of classes loaded from -Xbootclasspath/a, with the path entry they came from | ||
| readonly bootAppendPackages = new Map<string, string>(); | ||
| // Module objects defined to the boot loader, by name | ||
| readonly bootModules = new Map<string, JObject>(); | ||
| bootUnnamedModule: JObject | null = null; | ||
|
|
@@ -197,10 +207,12 @@ export class JVM { | |
| // Locale data is left out as --limit-modules would leave it, unless asked | ||
| // for, or named by --add-modules (by name or as ALL-SYSTEM). A limit given | ||
| // by the caller keeps it only if it names it, since no module requires it. | ||
| // With a module path it stays in: a limit would hide the modules the main | ||
| // module requires. | ||
| const addmods = [...this.props].filter(([k]) => k.startsWith("jdk.module.addmods.")).flatMap(([, v]) => v.split(",")); | ||
| const limit = this.props.get("jdk.module.limitmods"); | ||
| const noLocales = this.image !== null && (limit !== undefined ? !limit.split(",").includes("jdk.localedata") | ||
| : !opts.localeData && !addmods.some((m) => m === "ALL-SYSTEM" || m === "jdk.localedata")); | ||
| : !opts.localeData && !this.props.has("jdk.module.path") && !addmods.some((m) => m === "ALL-SYSTEM" || m === "jdk.localedata")); | ||
| const present = new Map<string, boolean>(); | ||
| for (const line of new TextDecoder().decode(index).split("\n")) { | ||
| const sp = line.indexOf(" "); | ||
|
|
@@ -250,8 +262,19 @@ export class JVM { | |
| } | ||
| const i = name.lastIndexOf("/"); | ||
| const module = this.bootPackages.get(i < 0 ? "" : name.slice(0, i)); | ||
| if (!module) return null; | ||
| const bytes = this.image ? this.image.read(module, `${name}.class`) : this.fs.read(`${this.opts.javaHome}/modules/${module}/${name}.class`); | ||
| // once the boot layer is defined, a module left out of it (--limit-modules) has no classes | ||
| if (module && this.bootModules.has("java.base") && !this.bootModules.has(module)) return null; | ||
| let bytes: Uint8Array | null | undefined; | ||
| if (module) { | ||
| bytes = this.opts.bootPatch?.(module, name) ?? (this.image ? this.image.read(module, `${name}.class`) : this.fs.read(`${this.opts.javaHome}/modules/${module}/${name}.class`)); | ||
| } | ||
| else { | ||
| const found = this.opts.bootClassPath?.(name); | ||
| if (found) { | ||
| bytes = found.bytes; | ||
| this.bootAppendPackages.set(i < 0 ? "" : name.slice(0, i), found.location); | ||
| } | ||
|
Comment on lines
+272
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Record the appended-package location only for classes that define successfully.
The package is also recorded with Proposed fix
|
||
| } | ||
| if (!bytes) return null; | ||
| const cf = parseClass(bytes); | ||
| const superclass = cf.superName ? this.loadBootOrFail(cf.superName) : null; | ||
|
|
@@ -715,7 +738,7 @@ export class JVM { | |
| const mod = this.bootPackages.get(c.packageName); | ||
| // a module not defined yet: defineModule0 sets it later, as HotSpot does | ||
| if (mod) return this.bootModules.get(mod) ?? null; | ||
| return this.bootUnnamedModule; | ||
| return this.bootModulePackages.get(c.packageName) ?? this.bootUnnamedModule; | ||
| } | ||
| const named = this.loaderData(c.loader).modules.get(c.packageName); | ||
| if (named) return named; | ||
|
|
@@ -880,6 +903,16 @@ export class JVM { | |
| this.throwNew(t, "java/lang/InternalError", e instanceof Error ? e.message : String(e)); | ||
| } | ||
|
|
||
| // Every live thread's Java stack, for kill -QUIT on a stuck program | ||
| dumpAllStacks() { | ||
| console.error(`Full thread dump ${this.props.get("java.vm.name") ?? "java-web"}:`); | ||
| for (const t of this.threads) { | ||
| if (t.state === TERMINATED) continue; | ||
| console.error(`\n"${t.name}" ${t.state === RUNNABLE ? "runnable" : "waiting"}`); | ||
| this.dumpStack(t); | ||
| } | ||
| } | ||
|
|
||
| dumpStack(t: JThread) { | ||
| for (let i = t.frames.length - 1; i >= 0; i--) { | ||
| const f = t.frames[i]; | ||
|
|
@@ -1167,6 +1200,10 @@ export class JVM { | |
| failed(e instanceof Error ? e : new Error(String(e))); | ||
| return; | ||
| } | ||
| if (this.exitCode !== null) { | ||
| failed(new Error(`VM exited during startup with ${this.exitCode}`)); | ||
| return; | ||
| } | ||
| if (this.opts.warmSnapshot && this.libraryStarting && this.snapshots) { | ||
| this.warmTraceActive = this.fs.beginWarmTrace(); | ||
| this.warmTraceClasses = this.classCount; | ||
|
|
@@ -1260,8 +1297,12 @@ export class JVM { | |
| "java/lang/invoke/MemberName", "java/lang/invoke/MethodHandleNatives", | ||
| ]) yield* init(n); | ||
| this.debug("initPhase2"); | ||
| const rc = yield t.call(boot("java/lang/System").methodMap.get("initPhase2(ZZ)I")!, 1, 1); | ||
| if (rc !== 0) throw new Error(`initPhase2 failed: ${rc}`); | ||
| // HotSpot prints a failed boot layer to stdout, without a stack trace, and exits | ||
| const rc = yield t.call(boot("java/lang/System").methodMap.get("initPhase2(ZZ)I")!, 0, 0); | ||
| if (rc !== 0) { | ||
| this.halt(1); | ||
| return; | ||
| } | ||
| this.debug("initPhase3"); | ||
| yield t.call(boot("java/lang/System").methodMap.get("initPhase3()V")!); | ||
| this.debug("vm started in", Date.now() - this.startTime, "ms"); | ||
|
|
@@ -1355,6 +1396,7 @@ export class JVM { | |
| if (!restored) { | ||
| if (cmd !== undefined) this.props.set("sun.java.command", ""); | ||
| yield* this.bootGen(t); | ||
| if (this.exitCode !== null) return; | ||
| // a snapshot is only a cache: failing to make or keep one never fails the run | ||
| if (snaps) { | ||
| try { | ||
|
|
@@ -1382,6 +1424,7 @@ export class JVM { | |
| private *mainGen(t: JThread, mode: number, what: string, args: string[], restored: boolean): NativeGen { | ||
| const boot = (n: string) => this.loadBootOrFail(n); | ||
| yield* this.startGen(t, restored); | ||
| if (this.exitCode !== null) return; | ||
| let code = 0; | ||
| try { | ||
| const helper = boot("sun/launcher/LauncherHelper"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated
bin/javascript assumesjava-webis on the PATH. In a packaged distribution or when the image is relocated,java-webmay not be in PATH, causing ProcessBuilder-launched child JVMs to fail. Use an absolute path to the java-web binary or a wrapper that locates it relative to java.home (e.g.,$(dirname "$0")/../lib/java-web).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 9d9b64a, this head: without
java-webon PATH,bin/javarunssrc/node.tsof the checkout the image's real path is in, and otherwise exits 127 with a message. ProcessBuilder never runs this script; it starts the VM's own java for that path.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix only works when the JDK image remains in the build directory (
build/jdk/bin/java→../../..finds the checkout withsrc/node.ts). In a packaged or relocated installation (e.g.,/opt/java-web/jdk/bin/java), the checkout won't be at that relative path andjava-webmay not be on PATH, so the script exits 127. The original concern stands: the wrapper should locate the launcher relative tojava.home(e.g.,$(dirname "$0")/../lib/java-web) rather than relying on PATH or the build layout.review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free