Refactor thread handling and improve memory management; add configura… #129

Stateford · 2026-01-19T02:22:19Z

…tion for testing

Copilot

Pull request overview

This PR refactors thread handling and improves memory management in the Display-Lock Windows application. It centralizes mutex management, adds proper error handling for memory operations, enhances thread safety, and includes configuration files for testing and AI assistance.

Changes:

Refactored thread closing functions to pass thread handles by pointer for proper cleanup
Improved memory management with proper realloc error handling and safer string operations (wcscpy -> wcscpy_s)
Centralized mutex lifecycle management in APPLICATION_ARGS structure
Added volatile qualifiers for thread-shared boolean flags
Removed unsafe _fcloseall() calls and improved file operation error handling
Added configuration files (CLAUDE.md, .claude/settings.local.json) and test resources

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/test_resources/test1.DLOCK	New test resource file for settings read tests
src/ui.c	Updated closeThread call to pass thread handle by pointer
src/main.c	Centralized mutex management, improved dialog creation error handling, added accelerator table cleanup, refactored closeThread calls
src/components/win.c	Added volatile qualifier to isRunning pointer for thread safety
src/components/update.c	Fixed realloc memory leak by using temporary pointer
src/components/settings.c	Replaced unsafe wcscpy with wcscpy_s, added fread error handling, removed _fcloseall()
src/components/menu.c	Updated closeThread to accept pointer to thread handle and volatile status pointer
src/components/applications.c	Enhanced memory safety with malloc/realloc checks, replaced wcscpy with wcscpy_s, removed _fcloseall(), improved variable naming, centralized mutex usage
include/menu.h	Updated closeThread function signature
include/common.h	Added magic number constants, updated closeThread signature, added volatile qualifiers, added mutex to APPLICATION_ARGS
include/applications.h	Updated closeApplicationThread function signature
CLAUDE.md	New documentation file for Claude Code AI assistant
.claude/settings.local.json	New configuration file for Claude Code permissions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T02:27:13Z

src/main.c

+        {
+            HWND hMainDlg = CreateDialog(NULL, MAKEINTRESOURCE(IDD_MAIN_VIEW), hWnd, MainWindow);
+            if (hMainDlg == NULL)
+            {
+                // Dialog creation failed
+                return -1;
+            }


The error handling returns -1 from a window procedure during WM_CREATE message. According to Windows API documentation, returning -1 from WM_CREATE will cause CreateWindow to fail, but the proper way to signal an error in WM_CREATE is to return -1, not from within a block. However, this return happens inside a scoped block and might not properly propagate. Consider using DestroyWindow and then returning -1, or restructuring the error handling.

Copilot · 2026-01-19T02:27:14Z

src/components/applications.c

    {
-        HANDLE mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);
-        WaitForSingleObject(mutex, INFINITE);
+        WaitForSingleObject(args->mutex, INFINITE);


Missing error handling for WaitForSingleObject. While INFINITE timeout is used, the function can still fail and return WAIT_FAILED. Consider checking the return value and handling errors appropriately to avoid undefined behavior if the wait operation fails.

Copilot · 2026-01-19T02:27:14Z

src/main.c

+                DWORD waitResult = WaitForSingleObject(args.mutex, INFINITE);

-                HANDLE mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);
-                DWORD result = WaitForSingleObject(mutex, INFINITE);
-
-                if ((result == 0) || (result == WAIT_ABANDONED))
+                if ((waitResult == WAIT_OBJECT_0) || (waitResult == WAIT_ABANDONED))
                {
-                    APPLICATION_SETTINGS *settings = &applicationList.applications[current_selected];
-                    DialogBoxParam(hInst, MAKEINTRESOURCE(IDC_APP_SETTINGS), hDlg, appSettingsProc, (LPARAM)settings);
+                    APPLICATION_SETTINGS *appSettings = &applicationList.applications[current_selected];
+                    DialogBoxParam(hInst, MAKEINTRESOURCE(IDC_APP_SETTINGS), hDlg, appSettingsProc, (LPARAM)appSettings);
                }
-                ReleaseMutex(mutex);
-                CloseHandle(mutex);
+                ReleaseMutex(args.mutex);


Potential undefined behavior if WaitForSingleObject fails. When the wait result is neither WAIT_OBJECT_0 nor WAIT_ABANDONED (e.g., WAIT_FAILED), ReleaseMutex is still called without having acquired the mutex, which will fail and could leave the application in an inconsistent state. Add an else clause or early return to skip ReleaseMutex when the wait operation doesn't succeed.

Copilot · 2026-01-19T02:27:14Z

src/main.c

+                DWORD waitResult = WaitForSingleObject(args.mutex, INFINITE);

-                HANDLE mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);
-                DWORD result = WaitForSingleObject(mutex, INFINITE);
-
-                if ((result == 0) || (result == WAIT_ABANDONED))
+                if ((waitResult == WAIT_OBJECT_0) || (waitResult == WAIT_ABANDONED))
                {
                    SendMessage(controls.listView, LB_DELETESTRING, current_selected, 0);
                    removeApplication(&applicationList, current_selected);
                }
-                ReleaseMutex(mutex);
-                CloseHandle(mutex);
+                ReleaseMutex(args.mutex);


Potential undefined behavior if WaitForSingleObject fails. When the wait result is neither WAIT_OBJECT_0 nor WAIT_ABANDONED (e.g., WAIT_FAILED), ReleaseMutex is still called without having acquired the mutex, which will fail and could leave the application in an inconsistent state. Add an else clause or early return to skip ReleaseMutex when the wait operation doesn't succeed.

Copilot · 2026-01-19T02:27:14Z

src/main.c

+                DWORD waitResult = WaitForSingleObject(args.mutex, INFINITE);

-                HANDLE mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);
-                DWORD result = WaitForSingleObject(mutex, INFINITE);
-
-                if ((result == 0) || (result == WAIT_ABANDONED))
+                if ((waitResult == WAIT_OBJECT_0) || (waitResult == WAIT_ABANDONED))
                {
-
                    APPLICATION_SETTINGS application;
                    ZeroMemory(&application, sizeof(application));
                    createApplicationSettings(ofn.lpstrFile, &application);

                    if (addApplication(&applicationList, application))
                        SendMessage(controls.listView, LB_ADDSTRING, 0, (LPARAM)application.application_name);
                }
-                ReleaseMutex(mutex);
-                CloseHandle(mutex);
+                ReleaseMutex(args.mutex);


Potential undefined behavior if WaitForSingleObject fails. When the wait result is neither WAIT_OBJECT_0 nor WAIT_ABANDONED (e.g., WAIT_FAILED), ReleaseMutex is still called without having acquired the mutex, which will fail and could leave the application in an inconsistent state. Add an else clause or early return to skip ReleaseMutex when the wait operation doesn't succeed.

Copilot · 2026-01-19T02:27:14Z

src/components/applications.c

 {
    // TODO: check better error checking
-    *thread = (HANDLE)_beginthreadex(NULL, 0, callback, args, 0, NULL);
+    *thread = (HANDLE)(uintptr_t)_beginthreadex(NULL, 0, callback, args, 0, NULL);


The cast through uintptr_t is good for suppressing warnings when converting between HANDLE and unsigned int, but the underlying issue is that _beginthreadex returns unsigned int, not HANDLE. Consider using a temporary variable to store the thread ID separately if needed, or document why this cast pattern is necessary.

Copilot · 2026-01-19T02:27:15Z

include/common.h

+// Magic number constants
+#define MAX_WINDOW_COUNT 35
+#define MAX_TITLE_LENGTH 500
+#define MAX_CLASS_NAME 500


The constant MAX_CLASS_NAME is defined but never used in the codebase. Consider removing it to keep the code clean, or document where it's intended to be used if it's for future functionality.

Suggested change

#define MAX_CLASS_NAME 500

Copilot · 2026-01-19T02:27:15Z

src/main.c

+        args.mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);
+        if (args.mutex == NULL)
+        {
+            // Mutex creation failed
+            return (INT_PTR)FALSE;


The static variable args is not properly initialized before WM_INITDIALOG. If WM_INITDIALOG is called multiple times (which can happen in Windows dialog lifecycle), the mutex will be recreated without properly cleaning up the previous handle, leading to a resource leak. Consider checking if args.mutex is already initialized before creating a new mutex, or ensure proper cleanup.

Suggested change

args.mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);

if (args.mutex == NULL)

{

// Mutex creation failed

return (INT_PTR)FALSE;

if (args.mutex == NULL)

{

args.mutex = CreateMutex(NULL, FALSE, APPLICATION_MUTEX_NAME);

if (args.mutex == NULL)

{

// Mutex creation failed

return (INT_PTR)FALSE;

}

Refactor thread handling and improve memory management; add configura…

dc06e6a

…tion for testing

Stateford self-assigned this Jan 19, 2026

Copilot AI review requested due to automatic review settings January 19, 2026 02:22

Copilot started reviewing on behalf of Stateford January 19, 2026 02:22 View session

Stateford merged commit 314aa84 into master Jan 19, 2026
5 of 9 checks passed

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor thread handling and improve memory management; add configura… #129

Refactor thread handling and improve memory management; add configura… #129

Uh oh!

Stateford commented Jan 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Refactor thread handling and improve memory management; add configura… #129

Refactor thread handling and improve memory management; add configura… #129

Uh oh!

Conversation

Stateford commented Jan 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants