Skip to content

Add a script to launch this app#3

Open
hjain-perforce wants to merge 3 commits into
NewSwavir:masterfrom
hjain-perforce:issue-1-1db10fac
Open

Add a script to launch this app#3
hjain-perforce wants to merge 3 commits into
NewSwavir:masterfrom
hjain-perforce:issue-1-1db10fac

Conversation

@hjain-perforce

Copy link
Copy Markdown

Auto-generated by P4-Integ-Orch for issue #1.

Good! Now let me create a summary of the fixes:

Summary of Fixes for Review Iteration 2

I've addressed all 4 blocker and major correctness issues:

1. [blocker/correctness] launch.bat:27 - Background server exits immediately

  • Fixed: Removed start /B and changed to run python -m http.server %PORT% in the foreground as the final command (line 58)
  • Server now blocks until Ctrl+C, properly serving requests throughout the script's lifetime

2. [blocker/correctness] launch.bat:33 - Race in port detection

  • Fixed: Completely rewrote the approach - now checks if port is in use BEFORE trying to start server (line 25)
  • Server only starts once a free port is confirmed
  • No longer starts server in background and checks after delay

3. [major/correctness] launch.sh:46 - False positive with kill -0 check

  • Fixed: Enhanced try_start_server() function to:
    • Start server and capture stderr to detect "Address already in use" errors
    • Use new check_port_listening() function to verify port is actually listening
    • Check both that process exists AND port is listening before considering success
    • Added retry logic with additional sleep if initial check is inconclusive

4. [major/correctness] launch.bat:38 - Unreliable taskkill with COMMANDLINE filter

  • Fixed: Removed all taskkill logic completely
  • New approach checks port availability BEFORE starting server, so no cleanup needed
  • Server runs in foreground, so it's automatically cleaned up when script exits

Additional Improvements:

  • launch.sh: Added check_port_listening() helper that tries multiple methods (lsof, ss, netstat with awk, /dev/tcp fallback) with proper port matching
  • launch.bat: Uses findstr /C:":%PORT% " (with trailing space) for exact port matching to avoid substring matches
  • Both scripts now start server in foreground as the final blocking command, ensuring proper lifecycle management

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