Skip to content

Commit 6282e67

Browse files
authored
fix(darwin): read keyboard layouts on the main thread (#1881)
1 parent ab41ff0 commit 6282e67

7 files changed

Lines changed: 221 additions & 76 deletions

File tree

‎docs/contributing/objective-c.md‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,5 +118,19 @@ if ([NSThread isMainThread]) {
118118
}
119119
```
120120

121-
Use `dispatch_sync` when the caller needs the result, and `dispatch_async` for
122-
UI updates and other non-blocking work.
121+
Use `dispatch_async` for UI updates and other work the caller does not wait
122+
for.
123+
124+
When a Go thread needs a result from the main thread, run the work inline if
125+
the caller is already on the main thread. Otherwise `dispatch_async` it to the
126+
main queue and wait on a semaphore with a timeout. A Go thread must never block
127+
on the main queue without a bound, because the main thread may be busy, or not
128+
running its loop at all, and the IPC or reload goroutine waiting on it would
129+
hang too. On timeout, leave the work queued and return an answer the caller
130+
can report as unknown or pending. Never run it on the calling thread instead,
131+
since the work is main-thread-only. `NeruSetReferenceKeyboardLayout` and
132+
`NeruCopyKeyboardLayouts` in `keymap_darwin.m` do this with a 500ms bound.
133+
134+
Never `dispatch_sync` onto the main queue from the main thread, because
135+
libdispatch traps it. The alert and overlay bridges still `dispatch_sync` for
136+
work the caller has to wait out, such as a modal alert.

‎internal/adapter/platform/darwin/keymap.go‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -93,25 +93,28 @@ func SetReferenceKeyboardLayout(inputSourceID string) bool {
9393
// find, which it decides itself because it also accepts localized names. It is
9494
// also true when the layout it found is no longer enabled, because the
9595
// resolver keeps its match after the user removes that layout.
96+
//
97+
// KeyboardLayouts reads the layouts on the main thread, because TIS must run
98+
// there. When the main queue does not answer within 500ms it returns no names
99+
// and no reference. The last result is then whether the resolver last failed
100+
// to find the configured layout, and false while an apply for another layout
101+
// is still queued, since that one has not been resolved yet.
96102
func KeyboardLayouts() ([]string, string, bool) {
97-
var names []string
98-
99-
if cIDs := C.NeruCopyKeyboardLayoutIDs(); cIDs != nil {
100-
if ids := C.GoString(cIDs); ids != "" {
101-
names = strings.Split(ids, "\n")
102-
}
103+
var cIDs, cReference *C.char
104+
var cResolved C.int
103105

104-
C.free(unsafe.Pointer(cIDs))
105-
}
106+
C.NeruCopyKeyboardLayouts(&cIDs, &cReference, &cResolved)
107+
defer C.free(unsafe.Pointer(cIDs))
108+
defer C.free(unsafe.Pointer(cReference))
106109

107-
var reference string
108-
109-
if cReference := C.NeruCopyReferenceKeyboardLayoutID(); cReference != nil {
110-
reference = C.GoString(cReference)
111-
C.free(unsafe.Pointer(cReference))
110+
var names []string
111+
if ids := C.GoString(cIDs); ids != "" {
112+
names = strings.Split(ids, "\n")
112113
}
113114

114-
unmatched := C.NeruReferenceKeyboardLayoutResolved() == 0 || !slices.Contains(names, reference)
115+
// With no names listed, only the resolver can say the layout is missing.
116+
reference := C.GoString(cReference)
117+
unmatched := cResolved == 0 || (len(names) > 0 && !slices.Contains(names, reference))
115118

116119
return names, reference, unmatched
117120
}

‎internal/adapter/platform/darwin/keymap.h‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,17 +196,16 @@ void NeruRefreshKeyboardLayoutMaps(void);
196196
/// applies once the main run loop runs it, logging a warning if it is missing.
197197
int NeruSetReferenceKeyboardLayout(const char *inputSourceID);
198198

199-
/// NeruCopyKeyboardLayoutIDs returns the input source IDs of the enabled
200-
/// keyboard layouts, one per line, or NULL. The caller frees it.
201-
char *NeruCopyKeyboardLayoutIDs(void);
202-
203-
/// NeruCopyReferenceKeyboardLayoutID returns the input source ID keys are
204-
/// named in, or NULL. The caller frees it.
205-
char *NeruCopyReferenceKeyboardLayoutID(void);
206-
207-
/// NeruReferenceKeyboardLayoutResolved returns 0 when kb_layout_to_use names a
208-
/// layout the resolver could not find, and 1 otherwise.
209-
int NeruReferenceKeyboardLayoutResolved(void);
199+
/// NeruCopyKeyboardLayouts reads the keyboard layouts through TIS on the main
200+
/// thread. It sets layoutIDsOut to the input source IDs of the enabled layouts,
201+
/// one per line, and referenceIDOut to the ID keys are named in. Either is NULL
202+
/// when unknown, and the caller frees both. resolvedOut is 0 when
203+
/// kb_layout_to_use names a layout the resolver could not find, and 1 otherwise.
204+
/// Off the main thread the call reads on the main queue. When that queue does
205+
/// not answer within 500ms the call leaves both IDs NULL and sets resolvedOut
206+
/// to what the main thread last recorded, or to 1 while a
207+
/// NeruSetReferenceKeyboardLayout apply for another layout is still queued.
208+
void NeruCopyKeyboardLayouts(char **layoutIDsOut, char **referenceIDOut, int *resolvedOut);
210209

211210
/// Callback type invoked after keyboard layout maps are rebuilt.
212211
typedef void (*KeymapLayoutChangeCallback)(void);

‎internal/adapter/platform/darwin/keymap_darwin.m‎

Lines changed: 74 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
static BOOL gUsesCurrentLayoutFallback = NO;
3636
/// whether the configured layout ID was resolved successfully (or no explicit ID was set)
3737
static BOOL gConfiguredInputSourceResolved = YES;
38+
/// layout ID the latest NeruSetReferenceKeyboardLayout call asked for, recorded
39+
/// before its apply is queued, so it can differ from gConfiguredInputSourceID
40+
static NSString *gRequestedInputSourceID = nil;
3841

3942
/// cached keycode-to-char maps for common modifier combinations (shift, caps, shift+caps)
4043
/// to avoid UCKeyTranslate calls during key event handling
@@ -1201,54 +1204,81 @@ void NeruRefreshKeyboardLayoutMaps(void) {
12011204
}
12021205
}
12031206

1204-
char *NeruCopyKeyboardLayoutIDs(void) {
1205-
@autoreleasepool {
1206-
NSDictionary *filter =
1207-
@{(__bridge NSString *)kTISPropertyInputSourceType : (__bridge NSString *)kTISTypeKeyboardLayout};
1208-
CFArrayRef inputSourceList = TISCreateInputSourceList((__bridge CFDictionaryRef)filter, false);
1209-
if (!inputSourceList) {
1210-
return NULL;
1211-
}
1207+
void NeruCopyKeyboardLayouts(char **layoutIDsOut, char **referenceIDOut, int *resolvedOut) {
1208+
*layoutIDsOut = NULL;
1209+
*referenceIDOut = NULL;
12121210

1213-
NSMutableArray<NSString *> *ids = [NSMutableArray array];
1214-
CFIndex sourceCount = CFArrayGetCount(inputSourceList);
1215-
for (CFIndex i = 0; i < sourceCount; i++) {
1216-
TISInputSourceRef source = (TISInputSourceRef)CFArrayGetValueAtIndex(inputSourceList, i);
1217-
NSString *sourceID = (__bridge NSString *)TISGetInputSourceProperty(source, kTISPropertyInputSourceID);
1218-
if (sourceID && inputSourceHasUnicodeLayoutData(source)) {
1219-
[ids addObject:sourceID];
1211+
__block NSString *layoutIDs = nil;
1212+
__block NSString *referenceID = nil;
1213+
__block BOOL resolved = YES;
1214+
1215+
void (^readLayouts)(void) = ^{
1216+
@autoreleasepool {
1217+
NSDictionary *filter =
1218+
@{(__bridge NSString *)kTISPropertyInputSourceType : (__bridge NSString *)kTISTypeKeyboardLayout};
1219+
CFArrayRef inputSourceList = TISCreateInputSourceList((__bridge CFDictionaryRef)filter, false);
1220+
if (inputSourceList) {
1221+
NSMutableArray<NSString *> *ids = [NSMutableArray array];
1222+
CFIndex sourceCount = CFArrayGetCount(inputSourceList);
1223+
for (CFIndex i = 0; i < sourceCount; i++) {
1224+
TISInputSourceRef source = (TISInputSourceRef)CFArrayGetValueAtIndex(inputSourceList, i);
1225+
NSString *sourceID =
1226+
(__bridge NSString *)TISGetInputSourceProperty(source, kTISPropertyInputSourceID);
1227+
if (sourceID && inputSourceHasUnicodeLayoutData(source)) {
1228+
[ids addObject:sourceID];
1229+
}
1230+
}
1231+
CFRelease(inputSourceList);
1232+
layoutIDs = [ids componentsJoinedByString:@"\n"];
1233+
}
1234+
1235+
TISInputSourceRef reference = copyResolvedReferenceInputSource(&resolved, NULL);
1236+
if (reference) {
1237+
referenceID =
1238+
[(__bridge NSString *)TISGetInputSourceProperty(reference, kTISPropertyInputSourceID) copy];
1239+
CFRelease(reference);
12201240
}
12211241
}
1222-
CFRelease(inputSourceList);
1242+
};
12231243

1224-
return strdup([[ids componentsJoinedByString:@"\n"] UTF8String]);
1225-
}
1226-
}
1244+
if ([NSThread isMainThread]) {
1245+
readLayouts();
1246+
} else {
1247+
// TIS must run on the main thread, so the read never falls back to
1248+
// this one. A main queue that does not answer in time leaves the read
1249+
// queued, and nobody reads its late results. The resolved flag the main
1250+
// thread last recorded needs no TIS, so the call still reports it.
1251+
dispatch_semaphore_t completed = dispatch_semaphore_create(0);
1252+
dispatch_async(dispatch_get_main_queue(), ^{
1253+
readLayouts();
1254+
dispatch_semaphore_signal(completed);
1255+
});
12271256

1228-
int NeruReferenceKeyboardLayoutResolved(void) {
1229-
@autoreleasepool {
1230-
BOOL resolved = YES;
1231-
TISInputSourceRef source = copyResolvedReferenceInputSource(&resolved, NULL);
1232-
if (source) {
1233-
CFRelease(source);
1234-
}
1257+
dispatch_time_t timeout = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(500 * NSEC_PER_MSEC));
1258+
if (dispatch_semaphore_wait(completed, timeout) != 0) {
1259+
NeruLog(NeruLogLevelWarn, @"Timed out waiting for the main queue, keyboard layouts not read", @"500ms");
1260+
1261+
// The flag belongs to the configured layout. While an apply for
1262+
// another layout is still queued it says nothing about the one
1263+
// requested, so the call reports that one as not unmatched.
1264+
[gKeymapLock lock];
1265+
BOOL applied = gRequestedInputSourceID == gConfiguredInputSourceID ||
1266+
[gRequestedInputSourceID isEqualToString:gConfiguredInputSourceID];
1267+
*resolvedOut = (!applied || gConfiguredInputSourceResolved) ? 1 : 0;
1268+
[gKeymapLock unlock];
12351269

1236-
return resolved ? 1 : 0;
1270+
return;
1271+
}
12371272
}
1238-
}
12391273

1240-
char *NeruCopyReferenceKeyboardLayoutID(void) {
12411274
@autoreleasepool {
1242-
TISInputSourceRef source = copyResolvedReferenceInputSource(NULL, NULL);
1243-
if (!source) {
1244-
return NULL;
1275+
if (layoutIDs) {
1276+
*layoutIDsOut = strdup([layoutIDs UTF8String]);
12451277
}
1246-
1247-
NSString *sourceID = (__bridge NSString *)TISGetInputSourceProperty(source, kTISPropertyInputSourceID);
1248-
char *copied = sourceID ? strdup([sourceID UTF8String]) : NULL;
1249-
CFRelease(source);
1250-
1251-
return copied;
1278+
if (referenceID) {
1279+
*referenceIDOut = strdup([referenceID UTF8String]);
1280+
}
1281+
*resolvedOut = resolved ? 1 : 0;
12521282
}
12531283
}
12541284

@@ -1265,11 +1295,15 @@ int NeruSetReferenceKeyboardLayout(const char *inputSourceID) {
12651295
}
12661296
}
12671297

1298+
initializeKeyMaps();
1299+
1300+
[gKeymapLock lock];
1301+
gRequestedInputSourceID = [trimmedInputSourceID copy];
1302+
[gKeymapLock unlock];
1303+
12681304
__block BOOL configuredResolved = YES;
12691305

12701306
void (^applyReferenceLayout)(void) = ^{
1271-
initializeKeyMaps();
1272-
12731307
[gKeymapLock lock];
12741308
gConfiguredInputSourceID = [trimmedInputSourceID copy];
12751309
clearResolvedReferenceInputSourceLocked();

‎internal/adapter/platform/darwin/keymap_integration_darwin_test.go‎

Lines changed: 92 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,18 +152,27 @@ func TestKeyboardLayouts_ListsTheEnabledLayoutsAndTheReference(t *testing.T) {
152152
// the main queue stalls. Resolving the layout calls TIS, which must run on the
153153
// main thread, so the apply waits in the queue and runs once the main thread
154154
// is free. The call returns true meanwhile, because a layout not yet applied
155-
// has not failed to resolve. An ID no layout has makes the apply visible on
156-
// any machine, since it flips the unmatched flag KeyboardLayouts reports.
155+
// has not failed to resolve. KeyboardLayouts reports the queued layout as not
156+
// unmatched too. The previous layout's outcome would fail doctor for a valid
157+
// replacement, and an outcome for the new one would mean TIS ran off the main
158+
// thread. An ID no layout has makes either mistake visible on any machine,
159+
// since it flips the unmatched flag KeyboardLayouts reports.
157160
func TestSetReferenceKeyboardLayout_LeavesTheApplyQueuedWhenTheMainQueueIsBlocked(t *testing.T) {
158161
const missingLayout = "com.example.keylayout.NoSuchLayout"
159162

163+
_, enabledLayout, _ := darwin.KeyboardLayouts()
164+
if enabledLayout == "" {
165+
t.Fatal("KeyboardLayouts named no reference layout")
166+
}
167+
160168
tests := []struct {
161169
name string
162170
prior string
163171
next string
164172
}{
165173
{name: "missing layout after auto", prior: "", next: missingLayout},
166174
{name: "auto after missing layout", prior: missingLayout, next: ""},
175+
{name: "enabled layout after missing layout", prior: missingLayout, next: enabledLayout},
167176
}
168177

169178
for _, test := range tests {
@@ -172,9 +181,8 @@ func TestSetReferenceKeyboardLayout_LeavesTheApplyQueuedWhenTheMainQueueIsBlocke
172181

173182
darwin.SetReferenceKeyboardLayout(test.prior)
174183

175-
_, _, priorUnmatched := darwin.KeyboardLayouts()
176-
if priorUnmatched != (test.prior != "") {
177-
t.Fatalf("unmatched is %v after setting %q", priorUnmatched, test.prior)
184+
if _, _, unmatched := darwin.KeyboardLayouts(); unmatched != (test.prior == missingLayout) {
185+
t.Fatalf("unmatched is %v after setting %q", unmatched, test.prior)
178186
}
179187

180188
release := darwin.BlockMainQueueForTesting()
@@ -194,24 +202,99 @@ func TestSetReferenceKeyboardLayout_LeavesTheApplyQueuedWhenTheMainQueueIsBlocke
194202
t.Fatalf("SetReferenceKeyboardLayout waited %v for the blocked main queue", elapsed)
195203
}
196204

197-
if _, _, unmatched := darwin.KeyboardLayouts(); unmatched != priorUnmatched {
198-
t.Fatal("layout was applied off the main thread while the main queue was blocked")
205+
if _, _, unmatched := darwin.KeyboardLayouts(); unmatched {
206+
t.Fatal("KeyboardLayouts reported a layout still queued as unmatched")
199207
}
200208

201209
release()
202210

211+
// A read that lists layouts ran on the main queue after the apply
212+
// queued before it, so its unmatched flag is the applied outcome.
203213
deadline := time.Now().Add(5 * time.Second)
204214
for {
205-
if _, _, unmatched := darwin.KeyboardLayouts(); unmatched != priorUnmatched {
215+
names, _, unmatched := darwin.KeyboardLayouts()
216+
if len(names) != 0 {
217+
if want := test.next == missingLayout; unmatched != want {
218+
t.Fatalf(
219+
"unmatched is %v after %q applied, want %v",
220+
unmatched,
221+
test.next,
222+
want,
223+
)
224+
}
225+
206226
return
207227
}
208228

209229
if time.Now().After(deadline) {
210-
t.Fatal("queued layout was never applied after the main queue drained")
230+
t.Fatal("KeyboardLayouts read nothing after the main queue was released")
211231
}
212232

213233
time.Sleep(50 * time.Millisecond)
214234
}
215235
})
216236
}
217237
}
238+
239+
// TestKeyboardLayouts_ReadsNothingOffTheMainThreadWhenTheMainQueueIsBlocked
240+
// pins that a caller off the main thread never reads the layouts through TIS
241+
// itself, which must run on the main thread. neru doctor and the health check
242+
// call it from the IPC goroutine. While the main queue is blocked it returns no
243+
// names and no reference, which doctor shows as not read yet. The unmatched
244+
// flag is the one the resolver last recorded. Only a TIS call produces the
245+
// names, so an empty list shows none ran here. Once the test releases the
246+
// queue, KeyboardLayouts reads the layouts again.
247+
func TestKeyboardLayouts_ReadsNothingOffTheMainThreadWhenTheMainQueueIsBlocked(t *testing.T) {
248+
_, _, wantUnmatched := darwin.KeyboardLayouts()
249+
250+
release := darwin.BlockMainQueueForTesting()
251+
t.Cleanup(release)
252+
253+
type layouts struct {
254+
names []string
255+
reference string
256+
unmatched bool
257+
}
258+
259+
result := make(chan layouts, 1)
260+
261+
go func() {
262+
names, reference, unmatched := darwin.KeyboardLayouts()
263+
result <- layouts{names: names, reference: reference, unmatched: unmatched}
264+
}()
265+
266+
var blocked layouts
267+
268+
select {
269+
case blocked = <-result:
270+
case <-time.After(2 * time.Second):
271+
t.Fatal("KeyboardLayouts waited for the blocked main queue")
272+
}
273+
274+
if len(blocked.names) != 0 || blocked.reference != "" {
275+
t.Fatalf(
276+
"KeyboardLayouts read %v and %q off the main thread",
277+
blocked.names,
278+
blocked.reference,
279+
)
280+
}
281+
282+
if blocked.unmatched != wantUnmatched {
283+
t.Fatalf(
284+
"unmatched is %v while the main queue is blocked, want %v",
285+
blocked.unmatched,
286+
wantUnmatched,
287+
)
288+
}
289+
290+
release()
291+
292+
names, reference, _ := darwin.KeyboardLayouts()
293+
if len(names) == 0 {
294+
t.Fatal("KeyboardLayouts read nothing after the main queue was released")
295+
}
296+
297+
if !slices.Contains(names, reference) {
298+
t.Fatalf("reference layout %q is not among the enabled layouts %v", reference, names)
299+
}
300+
}

‎internal/app/ipcctrl/info_health.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,12 @@ func capabilityStatusSupported(status string) bool {
282282
// layout kb_layout_to_use names, which is a typo neru doctor should catch.
283283
func keyboardLayoutsStatus(layouts ports.KeyboardLayouts, requested string) (string, bool) {
284284
if len(layouts.Names) == 0 {
285+
// The platform may know the layout is missing without listing any,
286+
// as macOS does when its main thread is too busy to list them.
287+
if requested != "" && layouts.Unmatched {
288+
return "not found: " + requested + " (layouts not read)", false
289+
}
290+
285291
// Doctor cannot check a forced layout here, so it does not call it ok.
286292
// It does not fail on it either, because the keymap may still arrive.
287293
if requested != "" {

0 commit comments

Comments
 (0)