From 99b9faa314cbec239d30fc3713ae5d3a15210494 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 28 Jan 2026 13:31:12 -0500 Subject: [PATCH 1/5] fix: Add unistd.h include for getpid() in Python POU compilation The iec_python.h header is included during compilation of generated PLC code. Python Function Blocks generate inline C code that calls getpid(), but the header only included sys/types.h (for pid_t) and was missing unistd.h (for getpid() declaration), causing compilation to fail with "implicit declaration of function 'getpid'" error. Co-Authored-By: Claude Opus 4.5 --- core/src/plc_app/include/iec_python.h | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/plc_app/include/iec_python.h b/core/src/plc_app/include/iec_python.h index 8afca3bb..aea3805d 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" From 4df63a0db4bfab580d0845f025a8e2bd80a96de2 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 28 Jan 2026 13:47:20 -0500 Subject: [PATCH 2/5] fix: Add cleanup for Python Function Blocks to prevent crash on stop The runtime crashed when stopping a PLC with Python Function Blocks that were actively printing to stdout. This happened because: 1. python_loader.c was compiled into libplc_*.so (not main executable) 2. Runner threads reading Python stdout were detached and never tracked 3. When dlclose() unloaded the library, runner threads crashed because their code (runner_thread function) was unmapped from memory Changes: - Track all Python blocks in an array with thread IDs, PIDs, and shm info - Use fork()/exec() instead of popen() to get the Python subprocess PID - Add python_blocks_cleanup() function that: - Sends SIGTERM to all Python processes (then SIGKILL if needed) - Joins all runner threads (they exit on EOF when Python dies) - Unmaps and unlinks shared memory regions - Removes Python script files - Call python_blocks_cleanup() before dlclose() in unload_plc_program() - Add null checks to LOG_INFO/LOG_ERROR macros for safety during cleanup Co-Authored-By: Claude Opus 4.5 --- core/src/plc_app/include/iec_python.h | 14 ++ core/src/plc_app/plc_state_manager.c | 12 + core/src/plc_app/python_loader.c | 302 ++++++++++++++++++++++---- 3 files changed, 283 insertions(+), 45 deletions(-) diff --git a/core/src/plc_app/include/iec_python.h b/core/src/plc_app/include/iec_python.h index aea3805d..e3e61114 100644 --- a/core/src/plc_app/include/iec_python.h +++ b/core/src/plc_app/include/iec_python.h @@ -81,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..d271e48f 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 32 + +// 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"); } From 83d07c26223d0d28dfabb84c8ca8b8bb8a7b9772 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 28 Jan 2026 13:48:58 -0500 Subject: [PATCH 3/5] refactor: Increase MAX_PYTHON_BLOCKS from 32 to 128 Co-Authored-By: Claude Opus 4.5 --- core/src/plc_app/python_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/plc_app/python_loader.c b/core/src/plc_app/python_loader.c index d271e48f..fa399ed3 100644 --- a/core/src/plc_app/python_loader.c +++ b/core/src/plc_app/python_loader.c @@ -61,7 +61,7 @@ static void (*py_log_error)(const char *fmt, ...); } while (0) // Maximum number of Python function blocks that can be loaded simultaneously -#define MAX_PYTHON_BLOCKS 32 +#define MAX_PYTHON_BLOCKS 128 // Tracking structure for each Python function block typedef struct From 44fc5acf2585decc1f69fd7d26b0c00a58bd06a6 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 28 Jan 2026 13:57:19 -0500 Subject: [PATCH 4/5] debug: Add logging to diagnose python_blocks_cleanup symbol lookup Co-Authored-By: Claude Opus 4.5 --- core/src/plc_app/plc_state_manager.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index 2fdea8af..7dceb53a 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -215,8 +215,13 @@ int unload_plc_program(PluginManager *pm) plugin_manager_get_symbol(pm, "python_blocks_cleanup"); if (python_cleanup) { + log_info("Calling python_blocks_cleanup()"); python_cleanup(); } + else + { + log_info("python_blocks_cleanup symbol not found (no Python FBs used)"); + } // Destroy the plugin manager plugin_manager_destroy(pm); From 3403de310a5cfb78d020128361ab47536eea0fcb Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 28 Jan 2026 14:08:06 -0500 Subject: [PATCH 5/5] refactor: Remove debug logging from Python cleanup Co-Authored-By: Claude Opus 4.5 --- core/src/plc_app/plc_state_manager.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/src/plc_app/plc_state_manager.c b/core/src/plc_app/plc_state_manager.c index 7dceb53a..2fdea8af 100644 --- a/core/src/plc_app/plc_state_manager.c +++ b/core/src/plc_app/plc_state_manager.c @@ -215,13 +215,8 @@ int unload_plc_program(PluginManager *pm) plugin_manager_get_symbol(pm, "python_blocks_cleanup"); if (python_cleanup) { - log_info("Calling python_blocks_cleanup()"); python_cleanup(); } - else - { - log_info("python_blocks_cleanup symbol not found (no Python FBs used)"); - } // Destroy the plugin manager plugin_manager_destroy(pm);