⋯ 55 lines Show the 55 lines above this hunk const capabilities = ; 56 56 const bridge_error = ; 57 57 const objc_runtime = ; 58 58 const ios_async = ; 59 + const compat = ; 59 60 const compat_mutex = ; 60 61 61 62 const objc = objc_runtime.objc; ⋯ 59 lines Show the 59 lines above this hunk pub const SiriBridge = struct { 121 122 bridge_error.sendResultToJS(self.allocator, A.register_siri_shortcut, out.items); 122 123 } 123 124 124 - /// Delete a donated activity, and answer when the deletion is done . 125 + /// Delete a donated activity, and answer when the deletion is done , or 126 + /// with `TIMEOUT` if it has not said so within `removal_deadline_ms`. 125 127 /// 126 128 /// `deleteSavedUserActivitiesWithPersistentIdentifiers:completionHandler:` 127 129 /// takes a `void (^)(void)`, and Swift replies from inside it. Replying 128 130 /// before it fired would tell the page a shortcut is gone while Siri can 129 131 /// still match it — the window is small and the claim is still false. 132 + /// 133 + /// The completion comes from a system daemon, and on a freshly booted 134 + /// simulator it sometimes never comes (#211). Nothing else would ever 135 + /// settle the call, so the deadline does, and never with `removed: true`: 136 + /// the deletion may or may not have happened, and a second removal is 137 + /// harmless. 138 + /// 139 + /// Every step that can fail comes before the reply slot is leased, except 140 + /// claiming a completion block, whose refusal the two `errdefer`s undo. 141 + /// Once a block holds the parked reply, nothing may fail until Foundation 142 + /// has it. 130 143 fn removeShortcut(self: *Self, data: []const u8) !void { 131 144 if (!is_darwin) return BridgeError.PlatformNotSupported; 132 145 ⋯ 3 lines Show the 3 lines above this hunk pub const SiriBridge = struct { moved, unchanged 136 149 const NSUserActivity = objc.objc_getClass("NSUserActivity") orelse 137 150 return BridgeError.PlatformNotSupported; 138 151 const identifiers = try singletonArray(self.allocator, fields.action); 152 + const sel = objc.sel_registerName( 153 + "deleteSavedUserActivitiesWithPersistentIdentifiers:completionHandler:", 154 + ) orelse return BridgeError.NativeCallFailed; 139 155 140 156 // The reply names the action, so it is shaped before the ticket exists 141 157 // and handed to the slot: the completion carries no arguments at all ⋯ 17 lines Show the 17 lines above this hunk pub const SiriBridge = struct { 159 175 }; 160 176 errdefer ios_async.abandon(ticket); 161 177 162 - publishPendingCall (ticket, reply) ; 178 + const block = claimBlock (ticket, reply) orelse { 179 + std.log.warn( 180 + "removeSiriShortcut: all {d} completion blocks are still owed a completion " ++ 181 + "Foundation never delivered; refusing rather than reusing one", 182 + .{block_count}, 183 + ); 184 + return BridgeError.NativeCallFailed; 185 + }; 186 + 187 + ios_async.scheduleDeadline(ticket, removal_deadline_ms, removalTimedOut); 163 188 164 - const sel = objc.sel_registerName( 165 - "deleteSavedUserActivitiesWithPersistentIdentifiers:completionHandler:", 166 - ) orelse return BridgeError.NativeCallFailed; 167 189 const DeleteFn = *const fn (objc.Class, objc.SEL, Id, *anyopaque) callconv(.c) void; 168 190 const deleteFn: DeleteFn = @ptrCast(&objc.objc_msgSend); 169 - deleteFn(NSUserActivity, sel, identifiers, doneBlock ( ticket )); 191 + deleteFn(NSUserActivity, sel, identifiers, @ptrCast ( &done_blocks[block] )); 170 192 } 171 193 }; 172 194 ⋯ 180 lines Show the 180 lines above this hunk fn release(object: Id) void { 353 375 // The deletion completion 354 376 // --------------------------------------------------------------------------- 355 377 356 - /// The reply a slot's completion will send . 378 + /// How long a removal waits for its completion before answering `TIMEOUT` . 379 + /// 380 + /// For `craft.siri.remove`, which arms no timeout of its own, this is the only 381 + /// thing that settles the call; for `_invoke` and `craft-bridge.js`, it is 382 + /// well under their 30 s, so the page hears the native answer rather than its 383 + /// own. It is the same budget the slice fixture 384 + /// gives a daemon-served location fix on the same simulator. A completion 385 + /// that arrives in time logs how long it took, so the number can be checked 386 + /// against CI. 387 + const removal_deadline_ms: u32 = 15_000; 388 + 389 + /// A removal waiting on its completion. 357 390 /// 358 391 /// `void (^)(void)` carries nothing — not even a success flag — so everything 359 392 /// the reply needs is shaped before the call and parked here. The ticket is 360 - /// stored rather than rebuilt from the slot index , because a `Ticket` is an 361 - /// index *and* a generation and inventing the generation would defeat the 362 - /// check that stops a late completion answering whoever holds the slot now. 393 + /// stored rather than rebuilt, because a `Ticket` is an index *and* a 394 + /// generation , and the deadline matches on both. 363 395 const PendingCall = struct { 364 396 ticket: ios_async.Ticket, 365 - /// Owned, `c_allocator`. Freed when the completion fires. 397 + /// Owned, `c_allocator`. Freed by whichever of the completion and the 398 + /// deadline takes the call. 366 399 reply: []u8, 400 + started_ms: i64, 401 + }; 402 + 403 + /// One per completion block, not per reply slot. 404 + /// 405 + /// A block knows only its own index. If blocks were chosen by reply slot, a 406 + /// deadline would release slot k, the next removal would lease slot k and get 407 + /// the same block, and the first removal's late completion would then answer 408 + /// the second with `removed: true` before its deletion had finished. So a 409 + /// block whose call the deadline answered stays `owed` until Foundation calls 410 + /// it, and is never handed to another call in the meantime. 411 + const BlockState = union(enum) { 412 + free, 413 + waiting: PendingCall, 414 + /// Answered by the deadline. Foundation still holds this block and may 415 + /// call it; when it does, that call is ignored and the block is freed. 416 + /// Carries when the call started, so a late completion can say how late. 417 + owed: i64, 367 418 }; 368 419 369 - var pending_calls: [ios_async.max_in_flight]?PendingCall = @splat(null); 420 + /// Twice the reply slots, so every slot can be waiting while as many blocks 421 + /// again are still owed a completion that has not come. 422 + const block_count = 2 * ios_async.max_in_flight; 423 + 424 + var blocks: [block_count]BlockState = @splat(.free); 370 425 var pending_mutex: compat_mutex.Mutex = .{}; 371 426 372 - fn publishPendingCall(ticket: ios_async.Ticket, reply: []u8) void { 427 + /// Park `reply` on the first free block. Null when every block is waiting or 428 + /// owed, which the caller refuses rather than reusing an owed block. 429 + fn claimBlock(ticket: ios_async.Ticket, reply: []u8) ?u5 { 373 430 pending_mutex.lock(); 374 431 defer pending_mutex.unlock(); 375 - pending_calls[ticket.index] = .{ .ticket = ticket, .reply = reply }; 432 + for (&blocks, 0..) |*state, i| { 433 + if (state.* != .free) continue; 434 + state.* = .{ .waiting = .{ .ticket = ticket, .reply = reply, .started_ms = compat.milliTimestamp() } }; 435 + return @intCast(i); 436 + } 437 + return null; 376 438 } 377 439 378 - /// Read and clear. Clearing is what makes a second fire of the same completion 379 - /// a no-op rather than a second reply and a double free. 380 - fn takePendingCall(index: u5) ?PendingCall { 440 + /// The deadline's half: take the call `ticket` names if it is still waiting, 441 + /// and leave its block owed. Matching the whole ticket is what stops a timer 442 + /// from expiring a newer call that leased the same reply slot. 443 + fn expireIfWaiting(ticket: ios_async.Ticket) ?PendingCall { 381 444 pending_mutex.lock(); 382 445 defer pending_mutex.unlock(); 383 - const call = pending_calls[index]; 384 - pending_calls[index] = null; 385 - return call; 446 + for (&blocks) |*state| { 447 + const call = switch (state.*) { 448 + .waiting => |call| call, 449 + else => continue, 450 + }; 451 + if (call.ticket.index != ticket.index or call.ticket.generation != ticket.generation) continue; 452 + state.* = .{ .owed = call.started_ms }; 453 + return call; 454 + } 455 + return null; 456 + } 457 + 458 + const Settled = union(enum) { 459 + /// The completion arrived first: answer with this call. 460 + reply: PendingCall, 461 + /// The deadline already answered. Nothing to send. When the call started. 462 + late: i64, 463 + /// No call was ever parked on this block. 464 + stray, 465 + }; 466 + 467 + /// The completion's half. Each block leaves `waiting` or `owed` exactly once, 468 + /// under the lock, so the completion and the deadline cannot both answer. 469 + fn settleBlock(block: u5) Settled { 470 + pending_mutex.lock(); 471 + defer pending_mutex.unlock(); 472 + const state = blocks[block]; 473 + blocks[block] = .free; 474 + return switch (state) { 475 + .waiting => |call| .{ .reply = call }, 476 + .owed => |started_ms| .{ .late = started_ms }, 477 + .free => .stray, 478 + }; 386 479 } 387 480 388 481 const BlockDescriptor = extern struct { ⋯ 19 lines Show the 19 lines above this hunk const done_block_descriptor = BlockDescriptor{ .size = @sizeOf(DoneBlock) }; 408 501 409 502 extern var _NSConcreteGlobalBlock: anyopaque; 410 503 411 - fn makeDoneInvoke(comptime index : u5) *const anyopaque { 504 + fn makeDoneInvoke(comptime block : u5) *const anyopaque { 412 505 const S = struct { 413 506 fn invoke(_: *const DoneBlock) callconv(.c) void { 414 - deletionFired( index ); 507 + deletionFired( block ); 415 508 } 416 509 }; 417 510 return @ptrCast(&S.invoke); 418 511 } 419 512 420 - fn makeDoneBlocks() [ ios_async.max_in_flight ]DoneBlock { 421 - var out: [ ios_async.max_in_flight ]DoneBlock = undefined; 513 + fn makeDoneBlocks() [ block_count ]DoneBlock { 514 + var out: [ block_count ]DoneBlock = undefined; 422 515 for (&out, 0..) |*b, i| { 423 516 b.* = .{ 424 517 .isa = &_NSConcreteGlobalBlock, ⋯ 5 lines Show the 5 lines above this hunk fn makeDoneBlocks() [ios_async.max_in_flight]DoneBlock { 430 523 return out; 431 524 } 432 525 433 - var done_blocks: [ ios_async.max_in_flight ]DoneBlock = 526 + var done_blocks: [ block_count ]DoneBlock = 434 527 if (is_darwin) makeDoneBlocks() else undefined; 435 528 436 - fn doneBlock(ticket: ios_async.Ticket) *anyopaque { 437 - return @ptrCast(&done_blocks[ticket.index]); 438 - } 439 - 440 529 /// Runs on whatever queue Foundation chose. `ios_async.deliverJson` copies the 441 530 /// payload and hops to the main queue, where `evaluateJavaScript:` is legal. 442 - fn deletionFired( index : u5) void { 531 + fn deletionFired( block : u5) void { 443 532 if (!is_darwin) return; 444 533 445 - const call = takePendingCall(index) orelse { 446 - std . log.warn( 447 - "removeSiriShortcut completion fired for slot {d} with no call recorded; ignored", 448 - . {index}, 449 - ); 450 - return; 451 - }; 534 + switch (settleBlock(block)) { 535 + . reply => |call| { 536 + defer std.heap.c_allocator.free(call.reply); 537 + std . log.info( 538 + "removeSiriShortcut: the deletion completion arrived after {d} ms", 539 + .{compat.milliTimestamp() - call.started_ms}, 540 + ); 541 + ios_async.deliverJson(call.ticket, call.reply); 542 + }, 543 + .late => |started_ms| std.log.warn( 544 + "removeSiriShortcut: a deletion completion arrived {d} ms after the call, after " ++ 545 + "its deadline had answered; ignored", 546 + .{compat.milliTimestamp() - started_ms}, 547 + ), 548 + .stray => std.log.warn( 549 + "removeSiriShortcut completion fired for block {d} with no call recorded; ignored", 550 + .{block}, 551 + ), 552 + } 553 + } 554 + 555 + /// On the main queue, `removal_deadline_ms` after the call. Does nothing when 556 + /// the completion got there first, which is the usual case: the timer cannot be 557 + /// cancelled. 558 + fn removalTimedOut(context: ?*anyopaque) callconv(.c) void { 559 + if (!is_darwin) return; 560 + 561 + const call = expireIfWaiting(ios_async.ticketFromDeadline(context)) orelse return; 452 562 defer std.heap.c_allocator.free(call.reply); 453 563 454 - ios_async.deliverJson(call.ticket, call.reply); 564 + // The page's message is fixed by the error code, so the framework is named 565 + // here. No `i=` in this line: the slice fixture greps for markers by it. 566 + std.log.warn( 567 + "removeSiriShortcut: +[NSUserActivity deleteSavedUserActivitiesWithPersistentIdentifiers:" ++ 568 + "completionHandler:] did not call its completion within {d} ms; answering TIMEOUT", 569 + .{removal_deadline_ms}, 570 + ); 571 + ios_async.deliverErrorCode(call.ticket, BridgeError.Timeout); 455 572 } 456 573 457 574 const testing = std.testing; ⋯ 68 lines Show the 68 lines above this hunk test "every declared action dispatches to something" { 526 643 527 644 test "a refused removal leases no reply slot" { 528 645 if (!is_darwin) return error.SkipZigTest; 646 + resetBlocks(); 529 647 530 648 var bridge = SiriBridge.init(testing.allocator); 531 649 defer bridge.deinit(); ⋯ 2 lines Show the 2 lines above this hunk test "a refused removal leases no reply slot" { 534 652 535 653 pending_mutex.lock(); 536 654 defer pending_mutex.unlock(); 537 - for ( pending_calls ) | entry | try testing.expect( entry == null ); 655 + for ( blocks ) | state | try testing.expect( state == .free ); 538 656 } 539 657 540 - test "each slot's completion block is global and has its own invoke" { 658 + test "each completion block is global and has its own invoke" { 541 659 if (!is_darwin) return error.SkipZigTest; 542 660 543 661 for (&done_blocks) |*b| { ⋯ 2 lines Show the 2 lines above this hunk test "each slot's completion block is global and has its own invoke" { 546 664 try testing.expectEqual(@sizeOf(DoneBlock), @as(usize, @intCast(b.descriptor.size))); 547 665 } 548 666 try testing.expect(done_blocks[0].invoke != done_blocks[1].invoke); 667 + try testing.expect(done_blocks[0].invoke != done_blocks[block_count - 1].invoke); 549 668 } 550 669 551 - test "a completion for a slot with no recorded call is ignored" { 670 + test "a completion for a block with no recorded call is ignored" { 552 671 // `void (^)(void)` carries nothing, so a stray fire has no way to identify 553 - // itself. Clearing the entry on take is what makes the second fire a 672 + // itself. Freeing the block on settle is what makes the second fire a 554 673 // no-op rather than a double free of the parked reply. 555 674 if (!is_darwin) return error.SkipZigTest; 675 + resetBlocks(); 676 + 677 + deletionFired(0); 678 + deletionFired(0); 679 + } 556 680 681 + // The deadline (#211). None of these reach `ios_async.deliver*`: they drive 682 + // the block table the deadline and the completion race over, which is where 683 + // "exactly one answer, to the right call" is decided. A host test has no main 684 + // queue for the timer or the hop. 685 + 686 + /// Free every block, and any reply a previous test left parked. 687 + fn resetBlocks() void { 557 688 pending_mutex.lock(); 558 - for (&pending_calls) |*entry| entry.* = null; 689 + defer pending_mutex.unlock(); 690 + for (&blocks) |*state| { 691 + switch (state.*) { 692 + .waiting => |call| std.heap.c_allocator.free(call.reply), 693 + else => {}, 694 + } 695 + state.* = .free; 696 + } 697 + } 698 + 699 + fn parkedReply(text: []const u8) ![]u8 { 700 + return std.heap.c_allocator.dupe(u8, text); 701 + } 702 + 703 + test "a deadline answers a waiting removal once, and leaves its block owed" { 704 + resetBlocks(); 705 + defer resetBlocks(); 706 + 707 + const ticket: ios_async.Ticket = .{ .index = 3, .generation = 7 }; 708 + const block = claimBlock(ticket, try parkedReply("{\"removed\":true}")) orelse return error.NoBlock; 709 + 710 + const call = expireIfWaiting(ticket) orelse return error.DeadlineFoundNothing; 711 + defer std.heap.c_allocator.free(call.reply); 712 + try testing.expectEqualStrings("{\"removed\":true}", call.reply); 713 + try testing.expect(blocks[block] == .owed); 714 + 715 + // A second expiry of the same call finds nothing to answer. 716 + try testing.expect(expireIfWaiting(ticket) == null); 717 + } 718 + 719 + test "a completion after its deadline is ignored, and frees its block" { 720 + resetBlocks(); 721 + defer resetBlocks(); 722 + 723 + const ticket: ios_async.Ticket = .{ .index = 3, .generation = 7 }; 724 + const block = claimBlock(ticket, try parkedReply("r")) orelse return error.NoBlock; 725 + const call = expireIfWaiting(ticket) orelse return error.DeadlineFoundNothing; 726 + std.heap.c_allocator.free(call.reply); 727 + 728 + try testing.expect(settleBlock(block) == .late); 729 + try testing.expect(blocks[block] == .free); 730 + try testing.expect(settleBlock(block) == .stray); 731 + } 732 + 733 + test "a deadline that fires after the completion does nothing" { 734 + // The usual case: the timer cannot be cancelled, so it fires for every 735 + // removal, including the ones that were answered long ago. 736 + resetBlocks(); 737 + defer resetBlocks(); 738 + 739 + const ticket: ios_async.Ticket = .{ .index = 3, .generation = 7 }; 740 + const block = claimBlock(ticket, try parkedReply("r")) orelse return error.NoBlock; 741 + switch (settleBlock(block)) { 742 + .reply => |call| std.heap.c_allocator.free(call.reply), 743 + else => return error.CompletionFoundNothing, 744 + } 745 + 746 + try testing.expect(expireIfWaiting(ticket) == null); 747 + try testing.expect(blocks[block] == .free); 748 + } 749 + 750 + test "a stale deadline cannot expire a newer removal on the same reply slot" { 751 + resetBlocks(); 752 + defer resetBlocks(); 753 + 754 + const first: ios_async.Ticket = .{ .index = 3, .generation = 7 }; 755 + const block = claimBlock(first, try parkedReply("first")) orelse return error.NoBlock; 756 + switch (settleBlock(block)) { 757 + .reply => |call| std.heap.c_allocator.free(call.reply), 758 + else => return error.CompletionFoundNothing, 759 + } 760 + 761 + const second: ios_async.Ticket = .{ .index = 3, .generation = 9 }; 762 + const again = claimBlock(second, try parkedReply("second")) orelse return error.NoBlock; 763 + 764 + try testing.expect(expireIfWaiting(first) == null); 765 + switch (blocks[again]) { 766 + .waiting => |call| try testing.expectEqual(@as(u32, 9), call.ticket.generation), 767 + else => return error.NewerCallWasExpired, 768 + } 769 + } 770 + 771 + test "a reply slot reused after a deadline never gets the block still owed" { 772 + // The mix-up a deadline keyed only by reply slot would ship: the first 773 + // removal's late completion answering the second with `removed: true` 774 + // before the second deletion had finished. 775 + resetBlocks(); 776 + defer resetBlocks(); 777 + 778 + const first: ios_async.Ticket = .{ .index = 0, .generation = 1 }; 779 + const first_block = claimBlock(first, try parkedReply("first")) orelse return error.NoBlock; 780 + const expired = expireIfWaiting(first) orelse return error.DeadlineFoundNothing; 781 + std.heap.c_allocator.free(expired.reply); 782 + 783 + // The same slot index, as `ios_async.acquire` hands out the lowest free one. 784 + const second: ios_async.Ticket = .{ .index = 0, .generation = 3 }; 785 + const second_block = claimBlock(second, try parkedReply("second")) orelse return error.NoBlock; 786 + try testing.expect(second_block != first_block); 787 + 788 + // The late completion for the first removal answers nobody. 789 + try testing.expect(settleBlock(first_block) == .late); 790 + switch (settleBlock(second_block)) { 791 + .reply => |call| { 792 + defer std.heap.c_allocator.free(call.reply); 793 + try testing.expectEqual(@as(u32, 3), call.ticket.generation); 794 + try testing.expectEqualStrings("second", call.reply); 795 + }, 796 + else => return error.SecondCallLost, 797 + } 798 + } 799 + 800 + test "with every block still owed, a removal is refused rather than reusing one" { 801 + resetBlocks(); 802 + defer resetBlocks(); 803 + 804 + pending_mutex.lock(); 805 + for (&blocks) |*state| state.* = .{ .owed = 0 }; 559 806 pending_mutex.unlock(); 560 807 561 - deletionFired(0); 562 - deletionFired(0); 808 + const reply = try parkedReply("r"); 809 + defer std.heap.c_allocator.free(reply); 810 + try testing.expect(claimBlock(.{ .index = 0, .generation = 1 }, reply) == null); 811 + for (blocks) |state| try testing.expect(state == .owed); 812 + } 813 + 814 + test "the deadline answers before the page's own request timeout" { 815 + // `craft-bridge.js` and `_invoke` give up after 30 s with a message that 816 + // names no framework and no code. (`craft.siri.remove` never gives up, so 817 + // for it this deadline is the only answer there is.) 818 + try testing.expect(removal_deadline_ms < 30_000); 563 819 }