Skip to content

Conversation

@ImDevinC
Copy link
Owner

Summary

This PR fixes critical issues with Discord socket connection handling and callback management that were causing connection failures and potential memory leaks.

Changes

Socket Connection Improvements:

  • Replace non-blocking socket operations with proper timeout-based approach
  • Add socket connection timeout (2s) for better error handling
  • Replace select() polling with socket.settimeout() for cleaner code
  • Use sendall() instead of manual chunked sending for reliability
  • Add SOCKET_BAD_BUFFER_SIZE error code to distinguish buffer issues from disconnects
  • Improve error handling during initial handshake phase

Callback Registration & Cleanup:

  • Register callbacks with both backend and plugin_base to prevent leaks
  • Implement clear_callbacks() method in main plugin class
  • Ensure callbacks are properly removed from both locations during cleanup
  • Add proper callback tracking in DiscordCore for lifecycle management

Technical Details

Socket Changes (discordrpc/sockets.py):

  • Connection now uses SOCK_STREAM explicitly with 2s timeout
  • Send operations use sendall() with 5s timeout
  • Receive operations use direct timeout (5s) instead of select()
  • Better buffer size validation with dedicated error code

Callback Changes (actions/DiscordCore.py, main.py):

  • Callbacks registered with plugin_base.add_callback() for global tracking
  • Cleanup calls plugin_base.remove_callback() to prevent orphaned callbacks
  • New clear_callbacks() method handles safe callback removal

Testing

These changes fix:

  • Socket connection timeouts and hangs
  • Race conditions in callback registration
  • Memory leaks from unreleased callbacks
  • Buffer size validation errors

Related

Builds on the performance improvements from PR #61

@ImDevinC ImDevinC merged commit 4328cd9 into main Dec 31, 2025
2 checks passed
@ImDevinC ImDevinC deleted the fix-socket-read branch December 31, 2025 03:55
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.

2 participants