keymanapp / keymanapp/keyman

chore(windows): audit usage of mutexes due to potential close-without-release errors

Open
#13,178 3 comments 0 reactions 1 assignee Claimed by @mcdurdin View on GitHub
chore windows/
Dominant language
Pascal
Stars
534
Forks
143
Avg merge
2d 10h
Merged PRs (30d)
113

Description

> I assume calling `CloseHandle` will also release the mutex?

I thought so, but when I read through Microsoft documentation, I realized I was wrong!

I wrote a tiny test harness (in Delphi because I was already there):

```delphi
program TestMutex;

{$APPTYPE CONSOLE}

uses
System.Classes,
System.SysUtils,
Winapi.Windows;

type
TMyThread = class(TThread)
procedure Execute; override;
end;

procedure TMyThread.Execute;
var
hMutex: THandle;
v: Cardinal;
begin
hMutex := CreateMutex(nil, False, 'MyTestMutexE');
if hMutex = 0 then
RaiseLastOSError;
v := WaitForSingleObject(hMutex, 0);
writeln('TMyThread: MyTestMutexE WFSO result: ' + IntToStr(v));
CloseHandle(hMutex);
end;

var
h: THandle;
t: TMyThread;
v: Cardinal;
begin
try
h := CreateMutex(nil, False, 'MyTestMutexE');
if h = 0 then
RaiseLastOSError;
t := TMyThread.Create(False);
t.WaitFor;
FreeAndNil(t);
v := WaitForSingleObject(h, 0);
writeln('MainThread: MyTestMutexE WFSO result: ' + IntToStr(v));
except
on E: Exception do
Writeln(E.ClassName, ': ', E.Message);
end;
readln;
end.
```

**Note:** `WaitForSingleObject` result values:
* 0 = WAIT_OBJECT_0
* 128 = WAIT_ABANDONED_0
* 258 = WAIT_TIMEOUt

This code gave the following result, indicating that the mutex was abandoned (rather than my assumption of WAIT_OBJECT_0):

```
TMyThread: MyTestMutexE WFSO result: 0
MainThread: MyTestMutexE WFSO result: 128
```

Dropping the `CloseHandle()` call and `FreeAndNil(t)` gave the following result of WAIT_TIMEOUT, as expected:

```
TMyThread: MyTestMutexE WFSO result: 0
MainThread: MyTestMutexE WFSO result: 258
```

So ... given my assumption here was wrong, I think I need to audit remaining usages of mutexes in Keyman for Windows.

_Originally posted by @mcdurdin in https://github.com/keymanapp/keyman/pull/13168#discussion_r1950200042_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.