Skip to content

Commit a88c73f

Browse files
committed
[sourcekit] Fix non-deterministic failure in CompileNotifications tests
This is a test-only change except for the introduction of a new request that is only used by tests. Our notifications are dispatched in the XPC event handler, which is not synchronized with replies to explicit XPC send_message_with_reply calls. This is fine for most users of sourcekitd, since the notifications are already enqueued on the client side, but for testing we need a way to guarantee that all notifications are passed to the client-side handler before we exit. This commit introduces a new request for testing that triggers a notification, allowing the client to wait on that notification to ensure all previously posted notifications have been handled. Note: the non-deterministic test failures can be triggered by adding a sleep of ~100 ms in the event handler before the notification is dispatched to the main queue. rdar://40311995
1 parent 197593b commit a88c73f

File tree

3 files changed

+42
-5
lines changed

3 files changed

+42
-5
lines changed

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,13 @@ static SourceKitRequest ActiveRequest = SourceKitRequest::None;
101101
static sourcekitd_uid_t SemaDiagnosticStage;
102102

103103
static sourcekitd_uid_t NoteDocUpdate;
104-
105104
static SourceKit::Semaphore semaSemaphore(0);
106105
static sourcekitd_response_t semaResponse;
107106
static const char *semaName;
108107

108+
static sourcekitd_uid_t NoteTest;
109+
static SourceKit::Semaphore noteSyncSemaphore(0);
110+
109111
namespace {
110112
struct AsyncResponseInfo {
111113
SourceKit::Semaphore semaphore{0};
@@ -139,7 +141,28 @@ struct NotificationBuffer {
139141
};
140142
static NotificationBuffer notificationBuffer;
141143

142-
static void printBufferedNotifications() {
144+
static void syncNotificationsWithService() {
145+
// Send TestNotification request, then wait for the notification. This ensures
146+
// that all notifications previously posted on the service side have been
147+
// passed to our notification handler.
148+
sourcekitd_object_t req = sourcekitd_request_dictionary_create(nullptr, nullptr, 0);
149+
sourcekitd_request_dictionary_set_uid(req, KeyRequest, RequestTestNotification);
150+
auto resp = sourcekitd_send_request_sync(req);
151+
if (sourcekitd_response_is_error(resp)) {
152+
sourcekitd_response_description_dump(resp);
153+
exit(1);
154+
}
155+
sourcekitd_response_dispose(resp);
156+
sourcekitd_request_release(req);
157+
if (noteSyncSemaphore.wait(60 * 1000)) {
158+
llvm::report_fatal_error("Test notification not received");
159+
}
160+
}
161+
162+
static void printBufferedNotifications(bool syncWithService = true) {
163+
if (syncWithService) {
164+
syncNotificationsWithService();
165+
}
143166
notificationBuffer.handleNotifications([](sourcekitd_response_t note) {
144167
sourcekitd_response_description_dump_filedesc(note, STDOUT_FILENO);
145168
});
@@ -171,6 +194,7 @@ static int skt_main(int argc, const char **argv) {
171194
SemaDiagnosticStage = sourcekitd_uid_get_from_cstr("source.diagnostic.stage.swift.sema");
172195

173196
NoteDocUpdate = sourcekitd_uid_get_from_cstr("source.notification.editor.documentupdate");
197+
NoteTest = sourcekitd_uid_get_from_cstr("source.notification.test");
174198

175199
#define REQUEST(NAME, CONTENT) Request##NAME = sourcekitd_uid_get_from_cstr(CONTENT);
176200
#define KIND(NAME, CONTENT) Kind##NAME = sourcekitd_uid_get_from_cstr(CONTENT);
@@ -342,11 +366,12 @@ static int handleTestInvocation(ArrayRef<const char *> Args,
342366

343367
assert(Opts.repeatRequest >= 1);
344368
for (unsigned i = 0; i < Opts.repeatRequest; ++i) {
345-
int ret = handleTestInvocation(Opts, InitOpts);
346-
printBufferedNotifications();
347-
if (ret) {
369+
if (int ret = handleTestInvocation(Opts, InitOpts)) {
370+
printBufferedNotifications(/*syncWithService=*/true);
348371
return ret;
349372
}
373+
// We will sync with the service before exiting; don't do so here.
374+
printBufferedNotifications(/*syncWithService=*/false);
350375
}
351376
return 0;
352377
}
@@ -1169,6 +1194,8 @@ static void notification_receiver(sourcekitd_response_t resp) {
11691194
semaResponse = sourcekitd_send_request_sync(edReq);
11701195
sourcekitd_request_release(edReq);
11711196
semaSemaphore.signal();
1197+
} else if (note == NoteTest) {
1198+
noteSyncSemaphore.signal();
11721199
} else {
11731200
notificationBuffer.add(resp);
11741201
}

tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,15 @@ void handleRequestImpl(sourcekitd_object_t ReqObj, ResponseReceiver Rec) {
322322
::exit(1);
323323
}
324324

325+
if (ReqUID == RequestTestNotification) {
326+
static UIdent TestNotification("source.notification.test");
327+
ResponseBuilder RespBuilder;
328+
auto Dict = RespBuilder.getDictionary();
329+
Dict.set(KeyNotification, TestNotification);
330+
sourcekitd::postNotification(RespBuilder.createResponse());
331+
return Rec(ResponseBuilder().createResponse());
332+
}
333+
325334
if (ReqUID == RequestDemangle) {
326335
SmallVector<const char *, 8> MangledNames;
327336
bool Failed = Req.getStringArray(KeyNames, MangledNames, /*isOptional=*/true);

utils/gyb_sourcekit_support/UIDs.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ def __init__(self, internal_name, external_name):
207207
REQUEST('SemanticRefactoring', 'source.request.semantic.refactoring'),
208208
REQUEST('EnableCompileNotifications',
209209
'source.request.enable-compile-notifications'),
210+
REQUEST('TestNotification', 'source.request.test_notification'),
210211
]
211212

212213

0 commit comments

Comments
 (0)