diff --git a/core/src/drivers/plugins/python/modbus_slave/simple_modbus.py b/core/src/drivers/plugins/python/modbus_slave/simple_modbus.py index 6c6c6f12..b56d36db 100644 --- a/core/src/drivers/plugins/python/modbus_slave/simple_modbus.py +++ b/core/src/drivers/plugins/python/modbus_slave/simple_modbus.py @@ -59,6 +59,93 @@ ) +class OpenPLCDeviceContext(ModbusDeviceContext): + """ + Custom Modbus device context that correctly handles FC5/FC6 response echo. + + Per Modbus specification, FC5 (Write Single Coil) and FC6 (Write Single Register) + responses should echo the requested value, not the current buffer state. + + pymodbus implements FC5/FC6 by calling setValues() then getValues() and using + the read-back value in the response. However, OpenPLC uses a journal-based write + system where writes are queued and applied at the next PLC scan cycle. This means + getValues() may return the old value before the journal write is applied. + + This class caches values written via FC5/FC6 and returns them on the subsequent + getValues() call, ensuring the response correctly echoes the requested value. + + The cache uses asyncio task ID to isolate concurrent requests, preventing race + conditions when multiple FC5/FC6 requests arrive simultaneously. + """ + + # Function codes that require echo response (write single operations) + _ECHO_FUNCTION_CODES = (5, 6) # FC5: Write Single Coil, FC6: Write Single Register + + def __init__(self, *args, **kwargs): + """Initialize with an empty pending writes cache.""" + super().__init__(*args, **kwargs) + # Cache for FC5/FC6 echo values: (task_id, func_code, address) -> value + self._pending_writes = {} + + def setValues(self, func_code, address, values): + """ + Set values in the datastore and cache for FC5/FC6 echo response. + + For FC5/FC6, the written values are cached with the current asyncio task ID + so that the subsequent getValues() call can return the correct echo value. + """ + # Cache values for FC5/FC6 echo response + if func_code in self._ECHO_FUNCTION_CODES: + try: + task_id = id(asyncio.current_task()) + for i, val in enumerate(values): + cache_key = (task_id, func_code, address + i) + self._pending_writes[cache_key] = val + except RuntimeError: + # No running event loop - skip caching (shouldn't happen in normal operation) + pass + + return super().setValues(func_code, address, values) + + def getValues(self, func_code, address, count=1): + """ + Get values from the datastore, using cached values for FC5/FC6 echo response. + + For FC5/FC6, if cached values exist for this task, they are returned and + removed from the cache. This ensures the response echoes the requested value + per Modbus specification. + """ + # For FC5/FC6, return cached echo values if available + if func_code in self._ECHO_FUNCTION_CODES: + try: + task_id = id(asyncio.current_task()) + results = [] + all_cached = True + + for i in range(count): + cache_key = (task_id, func_code, address + i) + if cache_key in self._pending_writes: + results.append(self._pending_writes.pop(cache_key)) + else: + all_cached = False + break + + if all_cached: + return results + + # If not all values were cached, clean up any partial cache entries + # and fall through to normal read + for i in range(len(results), count): + cache_key = (task_id, func_code, address + i) + self._pending_writes.pop(cache_key, None) + + except RuntimeError: + # No running event loop - fall through to normal read + pass + + return super().getValues(func_code, address, count) + + class OpenPLCCoilsDataBlock(ModbusSparseDataBlock): """Custom Modbus coils data block that mirrors OpenPLC bool_output using SafeBufferAccess""" @@ -996,7 +1083,8 @@ def init(args_capsule): ) # Create device context with all OpenPLC-connected data blocks - device = ModbusDeviceContext( + # Uses OpenPLCDeviceContext for correct FC5/FC6 response echo handling + device = OpenPLCDeviceContext( di=discrete_inputs_block, # Discrete Inputs -> bool_input (%IX) co=coils_block, # Coils -> bool_output (%QX) + bool_memory (%MX) ir=input_registers_block, # Input Registers -> int_input (%IW) diff --git a/core/src/plc_app/include/iec_python.h b/core/src/plc_app/include/iec_python.h index 8afca3bb..e3e61114 100644 --- a/core/src/plc_app/include/iec_python.h +++ b/core/src/plc_app/include/iec_python.h @@ -28,6 +28,7 @@ #include #include +#include #ifdef __cplusplus extern "C" @@ -80,6 +81,20 @@ extern "C" void python_loader_set_loggers(void (*log_info_func)(const char *, ...), void (*log_error_func)(const char *, ...)); + /** + * @brief Cleanup all Python function blocks + * + * This function must be called before unloading libplc.so to: + * 1. Terminate all Python subprocesses (SIGTERM, then SIGKILL) + * 2. Wait for all runner threads to exit + * 3. Unmap and unlink shared memory regions + * 4. Remove Python script files + * + * Failure to call this before dlclose() will cause a crash because + * the runner threads' code lives in the shared library. + */ + void python_blocks_cleanup(void); + #ifdef __cplusplus } #endif diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index bfa9d95c..2fdea8af 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -5,6 +5,7 @@ #include "image_tables.h" #include "journal_buffer.h" #include "plc_state_manager.h" +#include "plcapp_manager.h" #include "scan_cycle_manager.h" #include "utils/log.h" #include "utils/utils.h" @@ -206,6 +207,17 @@ int unload_plc_program(PluginManager *pm) image_tables_clear_null_pointers(); plugin_mutex_give(&plugin_driver->buffer_mutex); + // Cleanup Python function blocks BEFORE unloading the shared library + // This terminates Python subprocesses and joins runner threads to prevent + // crash when dlclose() unmaps the code while threads are still running + void (*python_cleanup)(void); + *(void **)(&python_cleanup) = + plugin_manager_get_symbol(pm, "python_blocks_cleanup"); + if (python_cleanup) + { + python_cleanup(); + } + // Destroy the plugin manager plugin_manager_destroy(pm); plc_program = NULL; diff --git a/core/src/plc_app/python_loader.c b/core/src/plc_app/python_loader.c index 1883a7b5..fa399ed3 100644 --- a/core/src/plc_app/python_loader.c +++ b/core/src/plc_app/python_loader.c @@ -29,11 +29,14 @@ #include #include #include +#include +#include #include #include #include #include #include +#include #include #include "include/iec_python.h" @@ -44,8 +47,43 @@ static void (*py_log_info)(const char *fmt, ...); static void (*py_log_error)(const char *fmt, ...); // Simple wrapper macros for logging -#define LOG_INFO(...) py_log_info(__VA_ARGS__) -#define LOG_ERROR(...) py_log_error(__VA_ARGS__) +#define LOG_INFO(...) \ + do \ + { \ + if (py_log_info) \ + py_log_info(__VA_ARGS__); \ + } while (0) +#define LOG_ERROR(...) \ + do \ + { \ + if (py_log_error) \ + py_log_error(__VA_ARGS__); \ + } while (0) + +// Maximum number of Python function blocks that can be loaded simultaneously +#define MAX_PYTHON_BLOCKS 128 + +// Tracking structure for each Python function block +typedef struct +{ + bool active; // Whether this slot is in use + pthread_t thread; // Runner thread ID + pid_t python_pid; // Python subprocess PID + int pipe_fd; // Pipe read end for stdout + void *shm_in_ptr; // Mapped input shared memory + void *shm_out_ptr; // Mapped output shared memory + size_t shm_in_size; // Size of input shared memory + size_t shm_out_size; // Size of output shared memory + char shm_in_name[256]; // Name of input shared memory region + char shm_out_name[256]; // Name of output shared memory region + char script_name[256]; // Python script filename +} python_block_t; + +// Array to track all Python blocks +static python_block_t python_blocks[MAX_PYTHON_BLOCKS]; +static int python_block_count = 0; +static pthread_mutex_t python_blocks_mutex = PTHREAD_MUTEX_INITIALIZER; +static volatile bool python_cleanup_in_progress = false; void python_loader_set_loggers(void (*log_info_func)(const char *, ...), void (*log_error_func)(const char *, ...)) @@ -55,28 +93,38 @@ void python_loader_set_loggers(void (*log_info_func)(const char *, ...), } /** - * @brief Thread function that runs the Python script and logs its output + * @brief Thread function that reads Python subprocess output and logs it * - * This function is spawned as a detached thread to run the Python interpreter - * and capture its stdout/stderr output for logging. + * This function reads from the pipe connected to Python's stdout/stderr + * and logs the output. It exits when the pipe is closed (Python exits or + * cleanup closes the pipe). * - * @param arg The command string to execute (will be freed by this function) + * @param arg Pointer to the python_block_t structure for this block * @return NULL */ static void *runner_thread(void *arg) { - const char *cmd = (const char *)arg; - FILE *fp = popen(cmd, "r"); + python_block_t *block = (python_block_t *)arg; + int pipe_fd = block->pipe_fd; + + // Convert pipe fd to FILE* for easier line reading + FILE *fp = fdopen(pipe_fd, "r"); if (fp == NULL) { - LOG_ERROR("[Python] Failed to start process: %s", cmd); - free((void *)cmd); + LOG_ERROR("[Python] Failed to fdopen pipe: %s", strerror(errno)); + close(pipe_fd); return NULL; } char buffer[512]; - while (fgets(buffer, sizeof(buffer), fp) != NULL) + while (!python_cleanup_in_progress && fgets(buffer, sizeof(buffer), fp) != NULL) { + // Check if cleanup started while we were blocked + if (python_cleanup_in_progress) + { + break; + } + // Remove trailing newline if present size_t len = strlen(buffer); if (len > 0 && buffer[len - 1] == '\n') @@ -86,8 +134,7 @@ static void *runner_thread(void *arg) LOG_INFO("[Python] %s", buffer); } - pclose(fp); - free((void *)cmd); + fclose(fp); // This also closes pipe_fd return NULL; } @@ -115,12 +162,36 @@ int python_block_loader(const char *script_name, const char *script_content, cha char shm_in_name[256]; char shm_out_name[256]; + // Find a free slot for this Python block + pthread_mutex_lock(&python_blocks_mutex); + int slot = -1; + for (int i = 0; i < MAX_PYTHON_BLOCKS; i++) + { + if (!python_blocks[i].active) + { + slot = i; + break; + } + } + if (slot == -1) + { + pthread_mutex_unlock(&python_blocks_mutex); + LOG_ERROR("[Python loader] Maximum number of Python blocks (%d) reached", + MAX_PYTHON_BLOCKS); + return -1; + } + python_block_t *block = &python_blocks[slot]; + memset(block, 0, sizeof(python_block_t)); + block->active = true; + python_block_count++; + pthread_mutex_unlock(&python_blocks_mutex); + // Write the Python script to disk FILE *fp = fopen(script_name, "w"); if (!fp) { LOG_ERROR("[Python loader] Failed to write Python script: %s", strerror(errno)); - return -1; + goto error_deactivate; } chmod(script_name, 0640); @@ -129,6 +200,11 @@ int python_block_loader(const char *script_name, const char *script_content, cha snprintf(shm_in_name, sizeof(shm_in_name), "%s_in", shm_name); snprintf(shm_out_name, sizeof(shm_out_name), "%s_out", shm_name); + // Store names for cleanup + strncpy(block->shm_in_name, shm_in_name, sizeof(block->shm_in_name) - 1); + strncpy(block->shm_out_name, shm_out_name, sizeof(block->shm_out_name) - 1); + strncpy(block->script_name, script_name, sizeof(block->script_name) - 1); + // Write script content with format specifiers replaced fprintf(fp, script_content, pid, shm_name, shm_name); fflush(fp); @@ -140,76 +216,212 @@ int python_block_loader(const char *script_name, const char *script_content, cha if (shm_in_fd < 0) { LOG_ERROR("[Python loader] shm_open (input) error: %s", strerror(errno)); - return -1; + goto error_deactivate; } if (ftruncate(shm_in_fd, shm_in_size) == -1) { LOG_ERROR("[Python loader] ftruncate (input) error: %s", strerror(errno)); close(shm_in_fd); - return -1; + goto error_deactivate; } *shm_in_ptr = mmap(NULL, shm_in_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_in_fd, 0); if (*shm_in_ptr == MAP_FAILED) { LOG_ERROR("[Python loader] mmap (input) error: %s", strerror(errno)); close(shm_in_fd); - return -1; + goto error_deactivate; } + close(shm_in_fd); + + // Store for cleanup + block->shm_in_ptr = *shm_in_ptr; + block->shm_in_size = shm_in_size; // Map shared memory for outputs int shm_out_fd = shm_open(shm_out_name, O_CREAT | O_RDWR, 0660); if (shm_out_fd < 0) { LOG_ERROR("[Python loader] shm_open (output) error: %s", strerror(errno)); - close(shm_in_fd); - munmap(*shm_in_ptr, shm_in_size); - shm_unlink(shm_in_name); - return -1; + goto error_cleanup_shm_in; } if (ftruncate(shm_out_fd, shm_out_size) == -1) { LOG_ERROR("[Python loader] ftruncate (output) error: %s", strerror(errno)); close(shm_out_fd); - close(shm_in_fd); - munmap(*shm_in_ptr, shm_in_size); - shm_unlink(shm_in_name); - return -1; + goto error_cleanup_shm_in; } *shm_out_ptr = mmap(NULL, shm_out_size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_out_fd, 0); if (*shm_out_ptr == MAP_FAILED) { LOG_ERROR("[Python loader] mmap (output) error: %s", strerror(errno)); close(shm_out_fd); - close(shm_in_fd); - munmap(*shm_in_ptr, shm_in_size); - shm_unlink(shm_in_name); - return -1; + goto error_cleanup_shm_in; } - - // Close file descriptors (mapping remains valid) - close(shm_in_fd); close(shm_out_fd); - // Prepare command to run Python script - char *cmd = malloc(512); - if (cmd == NULL) + // Store for cleanup + block->shm_out_ptr = *shm_out_ptr; + block->shm_out_size = shm_out_size; + + // Create pipe for Python stdout/stderr + int pipefd[2]; + if (pipe(pipefd) == -1) { - LOG_ERROR("[Python loader] malloc failed for cmd buffer"); - return -1; + LOG_ERROR("[Python loader] pipe() failed: %s", strerror(errno)); + goto error_cleanup_shm_out; } - snprintf(cmd, 512, "python3 -u %s 2>&1", script_name); - // Spawn thread to run Python process - pthread_t tid; - if (pthread_create(&tid, NULL, runner_thread, cmd) != 0) + // Fork to create Python subprocess + pid_t child_pid = fork(); + if (child_pid == -1) + { + LOG_ERROR("[Python loader] fork() failed: %s", strerror(errno)); + close(pipefd[0]); + close(pipefd[1]); + goto error_cleanup_shm_out; + } + + if (child_pid == 0) + { + // Child process - execute Python + close(pipefd[0]); // Close read end + + // Redirect stdout and stderr to pipe + dup2(pipefd[1], STDOUT_FILENO); + dup2(pipefd[1], STDERR_FILENO); + close(pipefd[1]); + + // Execute Python with unbuffered output + execlp("python3", "python3", "-u", script_name, (char *)NULL); + + // If exec fails + _exit(127); + } + + // Parent process + close(pipefd[1]); // Close write end + block->pipe_fd = pipefd[0]; + block->python_pid = child_pid; + + // Spawn thread to read Python output + if (pthread_create(&block->thread, NULL, runner_thread, block) != 0) { LOG_ERROR("[Python loader] pthread_create failed: %s", strerror(errno)); - free(cmd); - return -1; + close(pipefd[0]); + kill(child_pid, SIGKILL); + waitpid(child_pid, NULL, 0); + goto error_cleanup_shm_out; } - pthread_detach(tid); - LOG_INFO("[Python loader] Started Python function block: %s", script_name); + LOG_INFO("[Python loader] Started Python function block: %s (PID %d)", script_name, child_pid); return 0; + +error_cleanup_shm_out: + munmap(*shm_out_ptr, shm_out_size); + shm_unlink(shm_out_name); +error_cleanup_shm_in: + munmap(*shm_in_ptr, shm_in_size); + shm_unlink(shm_in_name); +error_deactivate: + pthread_mutex_lock(&python_blocks_mutex); + block->active = false; + python_block_count--; + pthread_mutex_unlock(&python_blocks_mutex); + return -1; +} + +void python_blocks_cleanup(void) +{ + LOG_INFO("[Python loader] Cleaning up %d Python function block(s)...", python_block_count); + + // Signal that cleanup is in progress - runner threads will check this flag + python_cleanup_in_progress = true; + + pthread_mutex_lock(&python_blocks_mutex); + + for (int i = 0; i < MAX_PYTHON_BLOCKS; i++) + { + python_block_t *block = &python_blocks[i]; + if (!block->active) + { + continue; + } + + LOG_INFO("[Python loader] Stopping Python block: %s (PID %d)", block->script_name, + block->python_pid); + + // Send SIGTERM to Python subprocess + if (block->python_pid > 0) + { + kill(block->python_pid, SIGTERM); + } + } + + pthread_mutex_unlock(&python_blocks_mutex); + + // Give Python processes time to exit gracefully + usleep(100000); // 100ms + + pthread_mutex_lock(&python_blocks_mutex); + + for (int i = 0; i < MAX_PYTHON_BLOCKS; i++) + { + python_block_t *block = &python_blocks[i]; + if (!block->active) + { + continue; + } + + // Check if process exited, if not send SIGKILL + if (block->python_pid > 0) + { + int status; + pid_t result = waitpid(block->python_pid, &status, WNOHANG); + if (result == 0) + { + // Process still running, force kill + LOG_INFO("[Python loader] Force killing Python block: %s (PID %d)", + block->script_name, block->python_pid); + kill(block->python_pid, SIGKILL); + waitpid(block->python_pid, NULL, 0); + } + } + + // Wait for runner thread to exit (it will get EOF from closed pipe) + pthread_join(block->thread, NULL); + + // Cleanup shared memory + if (block->shm_in_ptr && block->shm_in_size > 0) + { + munmap(block->shm_in_ptr, block->shm_in_size); + } + if (block->shm_out_ptr && block->shm_out_size > 0) + { + munmap(block->shm_out_ptr, block->shm_out_size); + } + if (block->shm_in_name[0] != '\0') + { + shm_unlink(block->shm_in_name); + } + if (block->shm_out_name[0] != '\0') + { + shm_unlink(block->shm_out_name); + } + + // Remove Python script file + if (block->script_name[0] != '\0') + { + unlink(block->script_name); + } + + block->active = false; + } + + python_block_count = 0; + python_cleanup_in_progress = false; + + pthread_mutex_unlock(&python_blocks_mutex); + + LOG_INFO("[Python loader] Cleanup complete"); }