Fixes from development branch#77
Merged
Merged
Conversation
Two issues fixed:
1. Deadlock with S7Comm plugin (plc_state_manager.c):
- unload_plc_program() tried to acquire buffer_mutex BEFORE stopping
plugins, but S7Comm's RWArea callback holds this mutex during
client reads, causing deadlock
- Fix: Call plugin_driver_stop() before acquiring the mutex
2. Hang when stopping without Python plugins (plugin_driver.c):
- plugin_driver_init/start/stop/destroy called Python GIL functions
even when no Python plugins exist or Python wasn't initialized
- Fix: Add has_python_plugin && Py_IsInitialized() checks before
all Python GIL operations
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unnecessary initialization of PyGILState_STATE to 0 (opaque type) - Add NULL check for main_tstate before PyEval_RestoreThread() to prevent undefined behavior if destroy is called without start Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use the explicit enum value PyGILState_LOCKED instead of 0 to satisfy compiler warning while being semantically correct. The variable is only used when have_gil/python_initialized is true, but the compiler's static analysis can't prove this across all control flow paths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fix: Prevent deadlock when stopping PLC with plugins
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves the safety and robustness of plugin driver lifecycle management, especially around the use of the Python Global Interpreter Lock (GIL) and destruction order. The changes ensure that Python-related operations are only performed when the Python interpreter is initialized, and that resource cleanup avoids potential deadlocks and undefined behavior.
Key improvements include:
Python GIL Management and Safety:
init,start,stop,restart, anddestroy) now check if Python is initialized (Py_IsInitialized()) before acquiring or releasing the GIL, preventing undefined behavior if Python is not available. [1] [2] [3] [4] [5] [6] [7]Resource Cleanup and Deadlock Prevention:
plugin_driver_destroy, the mutex and driver memory are freed early if there are no plugins, preventing resource leaks.unload_plc_program, the order of operations is changed to stop plugins before acquiring the buffer mutex, preventing a potential deadlock when plugins (like S7Comm) access the mutex during shutdown.Error Handling Improvements:
These changes collectively make the plugin driver more robust, especially in mixed native/Python plugin environments and in scenarios where the Python interpreter may or may not be initialized.