Fix cmux --version memory blowup (#1121)
* Add version memory guard regression test * Fix cmux --version plist lookup blowup
This commit is contained in:
parent
fc8142d61d
commit
4ba967556e
2 changed files with 238 additions and 24 deletions
|
|
@ -6720,15 +6720,12 @@ struct CMUXCLI {
|
||||||
}
|
}
|
||||||
|
|
||||||
private func versionInfoFromProjectFile() -> [String: String]? {
|
private func versionInfoFromProjectFile() -> [String: String]? {
|
||||||
guard let executable = currentExecutablePath(), !executable.isEmpty else {
|
guard let executableURL = resolvedExecutableURL() else {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
let fileManager = FileManager.default
|
let fileManager = FileManager.default
|
||||||
var current = URL(fileURLWithPath: executable)
|
var current = executableURL.deletingLastPathComponent()
|
||||||
.resolvingSymlinksInPath()
|
|
||||||
.standardizedFileURL
|
|
||||||
.deletingLastPathComponent()
|
|
||||||
|
|
||||||
while true {
|
while true {
|
||||||
let projectFile = current.appendingPathComponent("GhosttyTabs.xcodeproj/project.pbxproj")
|
let projectFile = current.appendingPathComponent("GhosttyTabs.xcodeproj/project.pbxproj")
|
||||||
|
|
@ -6820,23 +6817,29 @@ struct CMUXCLI {
|
||||||
}
|
}
|
||||||
|
|
||||||
private func candidateInfoPlistURLs() -> [URL] {
|
private func candidateInfoPlistURLs() -> [URL] {
|
||||||
guard let executable = currentExecutablePath(), !executable.isEmpty else {
|
guard let executableURL = resolvedExecutableURL() else {
|
||||||
return []
|
return []
|
||||||
}
|
}
|
||||||
|
|
||||||
let fileManager = FileManager.default
|
let fileManager = FileManager.default
|
||||||
let executableURL = URL(fileURLWithPath: executable)
|
|
||||||
.resolvingSymlinksInPath()
|
|
||||||
.standardizedFileURL
|
|
||||||
|
|
||||||
var candidates: [URL] = []
|
var candidates: [URL] = []
|
||||||
|
var seen: Set<String> = []
|
||||||
|
func appendIfExisting(_ url: URL) {
|
||||||
|
let path = url.path
|
||||||
|
guard !path.isEmpty else { return }
|
||||||
|
guard seen.insert(path).inserted else { return }
|
||||||
|
guard fileManager.fileExists(atPath: path) else { return }
|
||||||
|
candidates.append(url)
|
||||||
|
}
|
||||||
|
|
||||||
var current = executableURL.deletingLastPathComponent()
|
var current = executableURL.deletingLastPathComponent()
|
||||||
while true {
|
while true {
|
||||||
if current.pathExtension == "app" {
|
if current.pathExtension == "app" {
|
||||||
candidates.append(current.appendingPathComponent("Contents/Info.plist"))
|
appendIfExisting(current.appendingPathComponent("Contents/Info.plist"))
|
||||||
}
|
}
|
||||||
if current.lastPathComponent == "Contents" {
|
if current.lastPathComponent == "Contents" {
|
||||||
candidates.append(current.appendingPathComponent("Info.plist"))
|
appendIfExisting(current.appendingPathComponent("Info.plist"))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Local dev fallback: resolve version from the repo's app Info.plist
|
// Local dev fallback: resolve version from the repo's app Info.plist
|
||||||
|
|
@ -6845,7 +6848,7 @@ struct CMUXCLI {
|
||||||
let repoInfo = current.appendingPathComponent("Resources/Info.plist")
|
let repoInfo = current.appendingPathComponent("Resources/Info.plist")
|
||||||
if fileManager.fileExists(atPath: projectMarker.path),
|
if fileManager.fileExists(atPath: projectMarker.path),
|
||||||
fileManager.fileExists(atPath: repoInfo.path) {
|
fileManager.fileExists(atPath: repoInfo.path) {
|
||||||
candidates.append(repoInfo)
|
appendIfExisting(repoInfo)
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -6856,30 +6859,31 @@ struct CMUXCLI {
|
||||||
current = parent
|
current = parent
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If we already found an ancestor bundle or repo Info.plist, avoid scanning
|
||||||
|
// sibling app bundles. Large Resources directories can otherwise balloon RSS.
|
||||||
|
guard candidates.isEmpty else {
|
||||||
|
return candidates
|
||||||
|
}
|
||||||
|
|
||||||
let searchRoots = [
|
let searchRoots = [
|
||||||
executableURL.deletingLastPathComponent(),
|
executableURL.deletingLastPathComponent(),
|
||||||
executableURL.deletingLastPathComponent().deletingLastPathComponent()
|
executableURL.deletingLastPathComponent().deletingLastPathComponent()
|
||||||
]
|
]
|
||||||
for root in searchRoots {
|
for root in searchRoots {
|
||||||
guard let entries = try? fileManager.contentsOfDirectory(
|
guard let entries = fileManager.enumerator(
|
||||||
at: root,
|
at: root,
|
||||||
includingPropertiesForKeys: [.isDirectoryKey],
|
includingPropertiesForKeys: nil,
|
||||||
options: [.skipsHiddenFiles]
|
options: [.skipsHiddenFiles, .skipsSubdirectoryDescendants],
|
||||||
|
errorHandler: { _, _ in true }
|
||||||
) else {
|
) else {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
for entry in entries where entry.pathExtension == "app" {
|
for case let entry as URL in entries where entry.pathExtension == "app" {
|
||||||
candidates.append(entry.appendingPathComponent("Contents/Info.plist"))
|
appendIfExisting(entry.appendingPathComponent("Contents/Info.plist"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
var seen: Set<String> = []
|
return candidates
|
||||||
return candidates.filter { url in
|
|
||||||
let path = url.path
|
|
||||||
guard !path.isEmpty else { return false }
|
|
||||||
guard seen.insert(path).inserted else { return false }
|
|
||||||
return fileManager.fileExists(atPath: path)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private func currentExecutablePath() -> String? {
|
private func currentExecutablePath() -> String? {
|
||||||
|
|
@ -6897,6 +6901,20 @@ struct CMUXCLI {
|
||||||
return Bundle.main.executableURL?.path ?? args.first
|
return Bundle.main.executableURL?.path ?? args.first
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private func resolvedExecutableURL() -> URL? {
|
||||||
|
guard let executable = currentExecutablePath(), !executable.isEmpty else {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
let expanded = (executable as NSString).expandingTildeInPath
|
||||||
|
if let resolvedPath = realpath(expanded, nil) {
|
||||||
|
defer { free(resolvedPath) }
|
||||||
|
return URL(fileURLWithPath: String(cString: resolvedPath)).standardizedFileURL
|
||||||
|
}
|
||||||
|
|
||||||
|
return URL(fileURLWithPath: expanded).standardizedFileURL
|
||||||
|
}
|
||||||
|
|
||||||
private func usage() -> String {
|
private func usage() -> String {
|
||||||
return """
|
return """
|
||||||
cmux - control cmux via Unix socket
|
cmux - control cmux via Unix socket
|
||||||
|
|
|
||||||
196
tests/test_cli_version_memory_guard.py
Normal file
196
tests/test_cli_version_memory_guard.py
Normal file
|
|
@ -0,0 +1,196 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
"""
|
||||||
|
Regression test: `cmux --version` must not scan huge sibling app lists just to
|
||||||
|
resolve optional version metadata.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import glob
|
||||||
|
import os
|
||||||
|
import plistlib
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
import tempfile
|
||||||
|
import time
|
||||||
|
|
||||||
|
|
||||||
|
JUNK_APP_COUNT = 40000
|
||||||
|
RSS_LIMIT_KB = 64 * 1024
|
||||||
|
TIMEOUT_SECONDS = 10.0
|
||||||
|
EXPECTED_STDOUT = "cmux 9.9.9 (999)"
|
||||||
|
|
||||||
|
|
||||||
|
def resolve_cmux_cli() -> str:
|
||||||
|
explicit = os.environ.get("CMUX_CLI_BIN") or os.environ.get("CMUX_CLI")
|
||||||
|
if explicit and os.path.exists(explicit) and os.access(explicit, os.X_OK):
|
||||||
|
return explicit
|
||||||
|
|
||||||
|
candidates: list[str] = []
|
||||||
|
candidates.extend(glob.glob(os.path.expanduser("~/Library/Developer/Xcode/DerivedData/*/Build/Products/Debug/cmux")))
|
||||||
|
candidates.extend(glob.glob("/tmp/cmux-*/Build/Products/Debug/cmux"))
|
||||||
|
candidates = [p for p in candidates if os.path.exists(p) and os.access(p, os.X_OK)]
|
||||||
|
if candidates:
|
||||||
|
candidates.sort(key=os.path.getmtime, reverse=True)
|
||||||
|
return candidates[0]
|
||||||
|
|
||||||
|
in_path = shutil.which("cmux")
|
||||||
|
if in_path:
|
||||||
|
return in_path
|
||||||
|
|
||||||
|
raise RuntimeError("Unable to find cmux CLI binary. Set CMUX_CLI_BIN.")
|
||||||
|
|
||||||
|
|
||||||
|
def copy_runtime_frameworks(cli_path: str, fixture_contents: str) -> None:
|
||||||
|
frameworks_dir = os.path.join(fixture_contents, "Frameworks")
|
||||||
|
os.makedirs(frameworks_dir, exist_ok=True)
|
||||||
|
|
||||||
|
search_roots: list[str] = []
|
||||||
|
current = os.path.dirname(cli_path)
|
||||||
|
for _ in range(4):
|
||||||
|
search_roots.append(os.path.join(current, "Frameworks"))
|
||||||
|
search_roots.append(os.path.join(current, "PackageFrameworks"))
|
||||||
|
parent = os.path.dirname(current)
|
||||||
|
if parent == current:
|
||||||
|
break
|
||||||
|
current = parent
|
||||||
|
|
||||||
|
for search_root in search_roots:
|
||||||
|
sentry_framework = os.path.join(search_root, "Sentry.framework")
|
||||||
|
if os.path.isdir(sentry_framework):
|
||||||
|
shutil.copytree(sentry_framework, os.path.join(frameworks_dir, "Sentry.framework"))
|
||||||
|
return
|
||||||
|
|
||||||
|
|
||||||
|
def build_fixture(root: str, cli_path: str) -> str:
|
||||||
|
app_path = os.path.join(root, "cmux.app")
|
||||||
|
contents_path = os.path.join(app_path, "Contents")
|
||||||
|
resources_path = os.path.join(contents_path, "Resources")
|
||||||
|
bin_path = os.path.join(resources_path, "bin")
|
||||||
|
os.makedirs(bin_path, exist_ok=True)
|
||||||
|
|
||||||
|
fixture_cli = os.path.join(bin_path, "cmux")
|
||||||
|
shutil.copy2(cli_path, fixture_cli)
|
||||||
|
copy_runtime_frameworks(cli_path, contents_path)
|
||||||
|
|
||||||
|
info = {
|
||||||
|
"CFBundleExecutable": "cmux",
|
||||||
|
"CFBundleIdentifier": "test.cmux.version-memory-guard",
|
||||||
|
"CFBundlePackageType": "APPL",
|
||||||
|
"CFBundleShortVersionString": "9.9.9",
|
||||||
|
"CFBundleVersion": "999",
|
||||||
|
}
|
||||||
|
with open(os.path.join(contents_path, "Info.plist"), "wb") as handle:
|
||||||
|
plistlib.dump(info, handle)
|
||||||
|
|
||||||
|
for index in range(JUNK_APP_COUNT):
|
||||||
|
open(os.path.join(resources_path, f"junk-{index:05d}.app"), "wb").close()
|
||||||
|
|
||||||
|
return fixture_cli
|
||||||
|
|
||||||
|
|
||||||
|
def run_with_limits(cli_path: str, *args: str) -> dict[str, object]:
|
||||||
|
env = dict(os.environ)
|
||||||
|
env.pop("CMUX_COMMIT", None)
|
||||||
|
|
||||||
|
proc = subprocess.Popen(
|
||||||
|
[cli_path, *args],
|
||||||
|
stdout=subprocess.PIPE,
|
||||||
|
stderr=subprocess.PIPE,
|
||||||
|
text=True,
|
||||||
|
env=env,
|
||||||
|
)
|
||||||
|
|
||||||
|
started = time.time()
|
||||||
|
peak_rss_kb = 0
|
||||||
|
failure_reason: str | None = None
|
||||||
|
|
||||||
|
while True:
|
||||||
|
exit_code = proc.poll()
|
||||||
|
if exit_code is not None:
|
||||||
|
stdout, stderr = proc.communicate()
|
||||||
|
return {
|
||||||
|
"exit_code": exit_code,
|
||||||
|
"stdout": stdout.strip(),
|
||||||
|
"stderr": stderr.strip(),
|
||||||
|
"elapsed": time.time() - started,
|
||||||
|
"peak_rss_kb": peak_rss_kb,
|
||||||
|
"failure_reason": None,
|
||||||
|
}
|
||||||
|
|
||||||
|
try:
|
||||||
|
rss_kb = int(
|
||||||
|
subprocess.check_output(
|
||||||
|
["ps", "-o", "rss=", "-p", str(proc.pid)],
|
||||||
|
text=True,
|
||||||
|
).strip()
|
||||||
|
or "0"
|
||||||
|
)
|
||||||
|
except subprocess.CalledProcessError:
|
||||||
|
rss_kb = 0
|
||||||
|
|
||||||
|
peak_rss_kb = max(peak_rss_kb, rss_kb)
|
||||||
|
elapsed = time.time() - started
|
||||||
|
|
||||||
|
if rss_kb > RSS_LIMIT_KB:
|
||||||
|
failure_reason = f"rss limit exceeded ({rss_kb} KB > {RSS_LIMIT_KB} KB)"
|
||||||
|
elif elapsed > TIMEOUT_SECONDS:
|
||||||
|
failure_reason = f"timeout exceeded ({elapsed:.2f}s > {TIMEOUT_SECONDS:.2f}s)"
|
||||||
|
|
||||||
|
if failure_reason:
|
||||||
|
proc.kill()
|
||||||
|
stdout, stderr = proc.communicate()
|
||||||
|
return {
|
||||||
|
"exit_code": proc.returncode,
|
||||||
|
"stdout": stdout.strip(),
|
||||||
|
"stderr": stderr.strip(),
|
||||||
|
"elapsed": elapsed,
|
||||||
|
"peak_rss_kb": peak_rss_kb,
|
||||||
|
"failure_reason": failure_reason,
|
||||||
|
}
|
||||||
|
|
||||||
|
time.sleep(0.05)
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> int:
|
||||||
|
try:
|
||||||
|
cli_path = resolve_cmux_cli()
|
||||||
|
except Exception as exc:
|
||||||
|
print(f"FAIL: {exc}")
|
||||||
|
return 1
|
||||||
|
|
||||||
|
with tempfile.TemporaryDirectory(prefix="cmux-version-memory-guard-") as root:
|
||||||
|
fixture_cli = build_fixture(root, cli_path)
|
||||||
|
result = run_with_limits(fixture_cli, "--version")
|
||||||
|
|
||||||
|
if result["failure_reason"]:
|
||||||
|
print("FAIL: `cmux --version` exceeded runtime guard")
|
||||||
|
print(f"reason={result['failure_reason']}")
|
||||||
|
print(f"elapsed={result['elapsed']:.2f}s")
|
||||||
|
print(f"peak_rss_kb={result['peak_rss_kb']}")
|
||||||
|
print(f"stdout={result['stdout']}")
|
||||||
|
print(f"stderr={result['stderr']}")
|
||||||
|
return 1
|
||||||
|
|
||||||
|
if result["exit_code"] != 0:
|
||||||
|
print("FAIL: `cmux --version` exited non-zero")
|
||||||
|
print(f"exit={result['exit_code']}")
|
||||||
|
print(f"stdout={result['stdout']}")
|
||||||
|
print(f"stderr={result['stderr']}")
|
||||||
|
return 1
|
||||||
|
|
||||||
|
if result["stdout"] != EXPECTED_STDOUT:
|
||||||
|
print("FAIL: unexpected version output")
|
||||||
|
print(f"stdout={result['stdout']!r}")
|
||||||
|
print(f"expected={EXPECTED_STDOUT!r}")
|
||||||
|
return 1
|
||||||
|
|
||||||
|
print(
|
||||||
|
"PASS: `cmux --version` exits within memory/time limits "
|
||||||
|
f"(peak_rss_kb={result['peak_rss_kb']}, elapsed={result['elapsed']:.2f}s)"
|
||||||
|
)
|
||||||
|
return 0
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
raise SystemExit(main())
|
||||||
Loading…
Add table
Add a link
Reference in a new issue