Skip to content

Fix lock file cleanup when user triggers SIGINT through Ctrl-C#137

Merged
paolomainardi merged 2 commits into
masterfrom
copilot/fix-136
Aug 18, 2025
Merged

Fix lock file cleanup when user triggers SIGINT through Ctrl-C#137
paolomainardi merged 2 commits into
masterfrom
copilot/fix-136

Conversation

Copilot AI commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

When users press Ctrl-C during a sparkdock update process, the lock file /tmp/sparkdock.lock was not being properly cleaned up, leading to stale lock files that prevented subsequent runs.

Problem

The existing code only trapped the EXIT signal for lock file cleanup:

trap cleanup_lock EXIT

This meant that when users interrupted the process with Ctrl-C (SIGINT), the cleanup function wasn't called, leaving stale lock files. Users then had to manually remove the lock file:

rm -f /tmp/sparkdock.lock

Additionally, when stale lock files were detected, the script would exit with an error requiring manual intervention instead of automatically cleaning up and continuing.

Solution

  1. Enhanced signal handling: Extended the trap to catch SIGINT and SIGTERM in addition to EXIT:
trap cleanup_lock EXIT INT TERM
  1. Improved stale lock handling: Modified the check_update_lock() function to automatically remove stale lock files and continue execution:
# Before
print_warning "If you believe this is a stale lock file, you can remove it manually by running:"
print_warning "  rm -f $LOCK_FILE"
exit 1

# After  
print_warning "Removing stale lock file and continuing..."
rm -f "$LOCK_FILE"

Benefits

  • Users can safely press Ctrl-C to interrupt sparkdock without leaving stale locks
  • Automatic cleanup of stale locks means no manual intervention required
  • Improved user experience with clearer messaging
  • Maintains protection against concurrent updates

The changes are minimal (4 lines modified) and preserve all existing behavior while fixing the lock file cleanup issue.

Fixes #136.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


PR Type

Bug fix


Description

  • Enhanced signal handling to catch SIGINT and SIGTERM

  • Improved stale lock file detection and automatic cleanup

  • Removed manual intervention requirement for stale locks

  • Fixed lock file cleanup on Ctrl-C interruption


Changes walkthrough 📝

Relevant files
Bug fix
sparkdock.macos
Enhanced signal handling and stale lock cleanup                   

bin/sparkdock.macos

  • Enhanced trap to handle EXIT, INT, and TERM signals
  • Modified stale lock detection to automatically remove and continue
  • Removed manual cleanup instructions and error exit
  • Streamlined lock file management process
  • +4/-5     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Co-authored-by: paolomainardi <8747+paolomainardi@users.noreply.github.com>
    @paolomainardi paolomainardi marked this pull request as ready for review August 18, 2025 20:53
    Copilot AI review requested due to automatic review settings August 18, 2025 20:53

    Copilot AI left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    Copilot wasn't able to review any files in this pull request.


    You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

    @paolomainardi paolomainardi merged commit 3b75fa1 into master Aug 18, 2025
    3 checks passed
    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    136 - Fully compliant

    Compliant requirements:

    • Lock file must be deleted when user triggers SIGINT through Ctrl-C
    • Handle stale lock files more intelligently by detecting if process is actually running
    • If process is not running, automatically remove lock file and proceed instead of requiring manual intervention

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logic Gap

    The stale lock cleanup logic removes the lock file but doesn't handle the case where the lock file was created by a different process with the same PID that has since terminated. The cleanup should verify the lock file contains the current process PID before removal.

        print_warning "Lock file exists but the process (PID: $pid) is no longer running."
        print_warning "Removing stale lock file and continuing..."
        rm -f "$LOCK_FILE"
    fi

    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add error handling for lock removal

    The stale lock file removal should be logged more clearly and potentially include
    additional validation. Consider adding a confirmation message after successful
    removal and ensuring the lock file path is properly quoted.

    bin/sparkdock.macos [69-70]

     print_warning "Removing stale lock file and continuing..."
    -rm -f "$LOCK_FILE"
    +if rm -f "$LOCK_FILE"; then
    +    print "Stale lock file removed successfully"
    +else
    +    print_error "Failed to remove stale lock file: $LOCK_FILE"
    +    exit 1
    +fi
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion adds error handling for lock file removal which improves robustness, but the rm -f command rarely fails and the added complexity may be unnecessary for this use case.

    Low

    Copilot AI changed the title [WIP] Lock file must be deleted when user trigger a sigint through ctrl-c Fix lock file cleanup when user triggers SIGINT through Ctrl-C Aug 18, 2025
    Copilot AI requested a review from paolomainardi August 18, 2025 20:54
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    Lock file must be deleted when user trigger a sigint through ctrl-c

    3 participants