Skip to content

Fix orphaned dictionary generation processes - #68

Merged
barad1tos merged 3 commits into
mainfrom
fix/dictionary-process-teardown
Aug 14, 2026
Merged

barad1tos merged 3 commits into
mainfrom
fix/dictionary-process-teardown

Conversation

@barad1tos

@barad1tos barad1tos commented Aug 14, 2026 •

Copy link
Copy Markdown
Owner

── Summary ─────────────────────────────────

Dictionary generation commands could outlive a timeout or interruption, leaving their child processes running after the load had already failed. Terminate the full process tree on both paths while preserving the existing timeout result and interrupt handling contract.

── Changes ─────────────────────────────────

  • Fix: SdefDictionaryFileGenerator.kt — Kill dictionary command descendants and the parent on timeout or interruption, with a ProcessHandle fallback when the platform tree kill reports failure.
  • Test: DictionaryProcessTest.kt — Cover timeout, interruption, fallback teardown, and setup cleanup with real Unix process trees.

── Validation ────────────────────────────────

  • ./gradlew test koverXmlReport koverVerify verifyPlugin verifyPluginStructure detekt ktlintCheck --rerun-tasks --stacktrace — passed.
  • IntelliJ inspections for both touched Kotlin files — clean.
  • Plugin Verifier against IntelliJ Platform 251, 252, and 261 — compatible.

── Notes ────────────────────────────────────

The existing five-second timeout, typed load failure behavior, and worker interrupt restoration remain unchanged.

Summary by Sourcery

Ensure dictionary generation processes terminate fully on timeout or interruption to avoid orphaned child processes.

Bug Fixes:

  • Terminate dictionary generation process trees when commands time out or are interrupted, preventing orphaned subprocesses.

Tests:

  • Add Unix-based tests validating timeout, interruption, and fallback teardown for dictionary generation process trees.

- Stop dictionary command descendants after timeout or interruption
- Fall back to ProcessHandle teardown when the platform tree kill reports failure
- Cover timeout, interruption, and fallback cleanup with real process trees

Impact: Failed dictionary generation no longer leaves orphaned subprocesses

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @barad1tos, you have reached your weekly rate limit of 1500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR ensures that dictionary generation commands terminate their entire process tree on timeout or interruption, while preserving the existing timeout behavior and interrupt semantics, and adds Unix-based tests that validate these teardown paths including a fallback strategy when the platform tree-kill fails.

Sequence diagram for updated dictionary generation process teardown

sequenceDiagram
    participant SdefDictionaryFileGenerator
    participant Runtime
    participant Process
    participant OSProcessUtil
    participant ProcessHandle

    SdefDictionaryFileGenerator->>Runtime: exec(shellCommand)
    Runtime-->>SdefDictionaryFileGenerator: Process
    SdefDictionaryFileGenerator->>Process: waitForProcess(process, timeout, timeUnit)

    alt process finishes before timeout
        Process-->>SdefDictionaryFileGenerator: waitFor(timeout, timeUnit) == true
    else process times out
        Process-->>SdefDictionaryFileGenerator: waitFor(timeout, timeUnit) == false
        SdefDictionaryFileGenerator->>OSProcessUtil: killProcessTree(process)
        alt killProcessTree returns false
            SdefDictionaryFileGenerator->>Process: descendants()
            loop for each descendant
                SdefDictionaryFileGenerator->>ProcessHandle: destroyForcibly()
            end
            SdefDictionaryFileGenerator->>Process: destroyForcibly()
        end
    else InterruptedException thrown
        Process-->>SdefDictionaryFileGenerator: InterruptedException
        SdefDictionaryFileGenerator->>OSProcessUtil: killProcessTree(process)
        alt killProcessTree returns false
            SdefDictionaryFileGenerator->>Process: descendants()
            loop for each descendant
                SdefDictionaryFileGenerator->>ProcessHandle: destroyForcibly()
            end
            SdefDictionaryFileGenerator->>Process: destroyForcibly()
        end
        SdefDictionaryFileGenerator-->>SdefDictionaryFileGenerator: rethrow InterruptedException
    end
Loading

File-Level Changes

Change Details Files
Refactor dictionary generation to track the spawned process, centralize waiting logic, and ensure full process-tree termination on timeout or interruption with a ProcessHandle-based fallback.
  • Store the result of Runtime.exec in a Process variable instead of calling waitFor directly on the exec result.
  • Introduce a waitForProcess helper that wraps Process.waitFor with timeout, interruption propagation, and tree-termination logic.
  • Invoke terminateProcessTree when the process does not finish within the timeout or when InterruptedException is thrown.
  • Implement terminateProcessTree to first attempt OSProcessUtil.killProcessTree, and if that fails, forcibly kill all descendants via ProcessHandle.descendants before destroying the parent process.
src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt
Add Unix-only tests that spin up real parent/child process trees to verify timeout, interrupt, fallback kill behavior, and cleanup.
  • Create DictionaryProcessTest that exercises waitForProcess against a bash-created process tree with a long-running child.
  • Add testTimeoutKillsTree to assert that a timed-out waitForProcess returns false and stops both parent and child processes.
  • Add testInterruptKillsTree to assert that an interrupt causes waitForProcess to throw InterruptedException and that both processes are terminated.
  • Add testFallbackKillsTree to assert that when killProcessTree reports failure (simulated by always returning false), the fallback teardown still stops the entire process tree.
  • Implement test helpers to start a process tree via bash, capture the child PID in a temp file, resolve it to a ProcessHandle, and assert or enforce cleanup of all processes.
src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5690dcf437

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codecov

codecov Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.69565% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ng/dictionary/files/SdefDictionaryFileGenerator.kt 58.69% 7 Missing and 12 partials ⚠️

📢 Thoughts on this report? Let us know!

- Keep timeout and interruption outcomes when cleanup APIs fail
- Check every fallback termination request and log contextual cleanup issues
- Strengthen process tests for completion, timing, live interruption, and recursive trees

Impact: Dictionary load errors remain actionable without orphaning command processes
- Preserve descendant handles before a fallible platform tree kill can reparent them
- Add a real-process regression covering parent-first termination fallback

Impact: Dictionary timeouts and interruptions cannot leave reparented child processes running
@barad1tos
barad1tos merged commit 1a1f0ff into main Aug 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant