Skip to content

Add timeout and symbol length limits to demangling #540

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.

Sign up for GitHub

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

Merged
NicoHinderling merged 3 commits into from
Jan 15, 2026
Merged

Add timeout and symbol length limits to demangling #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d6f921
refactor to only be about timeout
NicoHinderling Jan 15, 2026
4336902
logging config
NicoHinderling Jan 15, 2026
2e1cf41
back chunk size with env var
NicoHinderling Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions src/launchpad/utils/apple/cwl_demangle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import shutil
import subprocess
import tempfile
import time
import uuid

from dataclasses import dataclass
Expand All @@ -13,6 +14,12 @@

logger = get_logger(__name__)

# Default timeout for cwl-demangle subprocess (in seconds)
DEFAULT_DEMANGLE_TIMEOUT = int(os.environ.get("LAUNCHPAD_DEMANGLE_TIMEOUT", "10"))

# Default chunk size for batching symbols
DEFAULT_CHUNK_SIZE = int(os.environ.get("LAUNCHPAD_DEMANGLE_CHUNK_SIZE", "500"))


@dataclass
class CwlDemangleResult:
Comment on lines +18 to 25
Copy link

@sentry sentry bot Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The default demangle timeout is 10 seconds, which contradicts the 30-second timeout mentioned in the pull request description.
Severity: HIGH

Suggested Fix

Align the code with the documented intention in the pull request description. Change the default value for LAUNCHPAD_DEMANGLE_TIMEOUT from "10" to "30" in cwl_demangle.py.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/launchpad/utils/apple/cwl_demangle.py#L18-L25

Potential issue: The default timeout for the demangle process is set to 10 seconds in
the code via `DEFAULT_DEMANGLE_TIMEOUT`, but the pull request description explicitly
states the intention was to add a 30-second timeout. This discrepancy means that symbol
demangling chunks that take between 10 and 30 seconds to process will time out
prematurely. When a timeout occurs, the function returns an empty dictionary, causing
the symbols in that chunk to be categorized as "Unattributed" instead of being correctly
demangled, potentially leading to less accurate symbol analysis.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down Expand Up @@ -75,7 +82,7 @@ def demangle_all(self) -> Dict[str, CwlDemangleResult]:
self.queue.clear()

# Process in chunks to avoid potential issues with large inputs
chunk_size = 5000
chunk_size = DEFAULT_CHUNK_SIZE
total_chunks = (len(names) + chunk_size - 1) // chunk_size

chunks: List[Tuple[List[str], int]] = []
Expand All @@ -84,7 +91,7 @@ def demangle_all(self) -> Dict[str, CwlDemangleResult]:
chunk_idx = i // chunk_size
chunks.append((chunk, chunk_idx))

# Only use parallel processing if workload justifies multiprocessing overhead (≥4 chunks = ≥20K symbols)
# Only use parallel processing if workload justifies multiprocessing overhead (≥4 chunks)
do_in_parallel = self.use_parallel and total_chunks >= 4

logger.debug(
Expand Down Expand Up @@ -149,6 +156,8 @@ def _demangle_chunk_worker(
if not chunk:
return {}

start_time = time.time()

binary_path = shutil.which("cwl-demangle")
if binary_path is None:
logger.error("cwl-demangle binary not found in PATH")
Expand Down Expand Up @@ -178,9 +187,16 @@ def _demangle_chunk_worker(
command_parts.append("--continue-on-error")

try:
result = subprocess.run(command_parts, capture_output=True, text=True, check=True)
result = subprocess.run(
command_parts, capture_output=True, text=True, check=True, timeout=DEFAULT_DEMANGLE_TIMEOUT
)
except subprocess.TimeoutExpired:
elapsed = time.time() - start_time
logger.exception("cwl-demangle subprocess timed out", extra={"chunk_idx": chunk_idx, "elapsed": elapsed})
return {}
except subprocess.CalledProcessError:
logger.exception(f"cwl-demangle failed for chunk {chunk_idx}")
elapsed = time.time() - start_time
logger.exception("cwl-demangle subprocess failed", extra={"chunk_idx": chunk_idx, "elapsed": elapsed})
return {}

batch_result = json.loads(result.stdout)
Expand Down
15 changes: 15 additions & 0 deletions tests/integration/test_cwl_demangle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,21 @@ def test_environment_variable_disables_parallel(self):
demangler = CwlDemangler()
assert demangler.use_parallel is False

def test_timeout_configuration(self):
"""Test LAUNCHPAD_DEMANGLE_TIMEOUT env var configures timeout."""
demangler = CwlDemangler()

# Test with custom timeout
with mock.patch.dict(os.environ, {"LAUNCHPAD_DEMANGLE_TIMEOUT": "10"}):
# Generate a few symbols to trigger sequential processing
symbols = self._generate_symbols(100)
for symbol in symbols:
demangler.add_name(symbol)

result = demangler.demangle_all()
# Should succeed with custom timeout
assert len(result) == 100

def _generate_symbols(self, count: int) -> list[str]:
"""Generate valid Swift mangled symbols."""
letters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
Expand Down
Loading