119767 2013-08-13 13:49:44 -0700 [WK2] Sync messages with non-default timeout sent from secondary threads always time out 2013-08-14 14:35:48 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Mac OS X 10.8 RESOLVED FIXED InRadar P2 Major --- 0 justin webkit-unassigned commit-queue oldest_to_newest 916862 0 208670 justin 2013-08-13 13:49:44 -0700 Created attachment 208670 proposed patch Any request from the UI process to the Web Content process that is made from a secondary thread will probably fail, because of a bug introduced with Connection::sendSyncMessageFromSecondaryThread() in r139514 (https://bugs.webkit.org/show_bug.cgi?id=106708). It is quite simple: the timeout argument to semaphore.wait() is supposed to be an absolute time, but the length of the timeout is being sent instead. 916881 1 ap 2013-08-13 14:17:42 -0700 <rdar://problem/14578770> Thank you, this is a good catch. Note however that we never send sync messages with timeouts from secondary threads. Can you please tell us more about the failing scenario? 916885 2 208676 ap 2013-08-13 14:24:23 -0700 Created attachment 208676 patch with ChangeLog 916926 3 justin 2013-08-13 15:54:06 -0700 The timeout comes from ChildProcessProxy.h (default argument in ChildProcessProxy::sendSync(), called from UIProcess/mac/WebPageProxyMac.mm). The use case is calling methods of the NSTextInputClient protocol, such as - attributedSubstringForProposedRange:actualRange: on WKView. The secondary thread is running my scripting addition. More details on my scenario (a grammar and spell check application) in bug 61889. It would probably be possible to do the same work using the AppleScript command "do JavaScript" to access the DOM directly. The nice thing about NSTextInputClient is that it conveniently converts back and forth between the DOM and flat attributed strings. 916950 4 ap 2013-08-13 17:05:31 -0700 Thank you for the additional information! It's not valid to use NSTextInputClient methods form a secondary thread. We make no attempt to make this work, and in fact do access a lot of shared state from these methods. We should still fix this mistake of course, however I strongly suggest looking into a way to not call -attributedSubstringForProposedRange:actualRange: from secondary threads. 916962 5 justin 2013-08-13 17:27:16 -0700 (In reply to comment #4) Ah, that is good to know. I will make my code comply with that restriction. Thank you! 917292 6 208676 commit-queue 2013-08-14 14:35:46 -0700 Comment on attachment 208676 patch with ChangeLog Clearing flags on attachment: 208676 Committed r154073: <http://trac.webkit.org/changeset/154073> 917293 7 commit-queue 2013-08-14 14:35:48 -0700 All reviewed patches have been landed. Closing bug. 208670 2013-08-13 13:49:44 -0700 2013-08-13 14:24:23 -0700 proposed patch CoreIPC-Connection.cpp.patch text/plain 494 justin LS0tIFdlYktpdC1yMTUzODgyL1NvdXJjZS9XZWJLaXQyL1BsYXRmb3JtL0NvcmVJUEMvQ29ubmVj dGlvbi5jcHAJMjAxMy0wNy0zMCAyMToxMzoyMS4wMDAwMDAwMDAgLTA0MDAKKysrIFdlYktpdC1y MTUzODgyL1NvdXJjZS9XZWJLaXQyL1BsYXRmb3JtL0NvcmVJUEMvQ29ubmVjdGlvbisuY3BwCTIw MTMtMDgtMTMgMTY6MzA6MzUuMDAwMDAwMDAwIC0wNDAwCkBAIC01MTEsNyArNTExLDkgQEAKICAg ICBpZiAodGltZW91dCA9PSBOb1RpbWVvdXQpCiAgICAgICAgIHRpbWVvdXQgPSAxZTEwOwogCi0g ICAgcGVuZGluZ1JlcGx5LnNlbWFwaG9yZS53YWl0KHRpbWVvdXQpOworICAgIGRvdWJsZSBhYnNv bHV0ZVRpbWUgPSBjdXJyZW50VGltZSgpICsgdGltZW91dDsKKworICAgIHBlbmRpbmdSZXBseS5z ZW1hcGhvcmUud2FpdChhYnNvbHV0ZVRpbWUpOwogCiAgICAgLy8gRmluYWxseSwgcG9wIHRoZSBw ZW5kaW5nIHN5bmMgcmVwbHkgaW5mb3JtYXRpb24uCiAgICAgewo= 208676 2013-08-13 14:24:23 -0700 2013-08-14 14:35:46 -0700 patch with ChangeLog timeout.txt text/plain 1367 ap SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NDAyMikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEzLTA4LTEzICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIFtXSzJdIFN5bmMgbWVzc2Fn ZXMgd2l0aCB3aXRoIG5vbi1kZWZhdWx0IHRpbWVvdXQgc2VudCBmcm9tIHNlY29uZGFyeSB0aHJl YWRzIGFsd2F5cyB0aW1lIG91dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTE5NzY3CisKKyAgICAgICAgRml4IGJ5IEp1c3RpbiBCdXIuCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogUGxhdGZvcm0vQ29yZUlQ Qy9Db25uZWN0aW9uLmNwcDogKENvcmVJUEM6OkNvbm5lY3Rpb246OnNlbmRTeW5jTWVzc2FnZUZy b21TZWNvbmRhcnlUaHJlYWQpOgorICAgICAgICBUaGUgYXJndW1lbnQgdG8gd2FpdCgpIGlzIGFi c29sdXRlIHRpbWUsIG5vdCBhIGRlbHRhLiBUaGlzIGNhbGN1bGF0aW9uIG1hdGNoZXMgb25lCisg ICAgICAgIGluIENvbm5lY3Rpb246OndhaXRGb3JNZXNzYWdlKCkuCisKIDIwMTMtMDgtMTIgIERh biBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBGaXhlZCBhbiBvZmYtYnkt KHNpemVvZihjaGFyKikgLSAxKSBidWcuCkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9QbGF0Zm9ybS9D b3JlSVBDL0Nvbm5lY3Rpb24uY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1BsYXRm b3JtL0NvcmVJUEMvQ29ubmVjdGlvbi5jcHAJKHJldmlzaW9uIDE1NDAwNikKKysrIFNvdXJjZS9X ZWJLaXQyL1BsYXRmb3JtL0NvcmVJUEMvQ29ubmVjdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTUxMSw3ICs1MTEsNyBAQCBQYXNzT3duUHRyPE1lc3NhZ2VEZWNvZGVyPiBDb25uZWN0aW9uOjpz CiAgICAgaWYgKHRpbWVvdXQgPT0gTm9UaW1lb3V0KQogICAgICAgICB0aW1lb3V0ID0gMWUxMDsK IAotICAgIHBlbmRpbmdSZXBseS5zZW1hcGhvcmUud2FpdCh0aW1lb3V0KTsKKyAgICBwZW5kaW5n UmVwbHkuc2VtYXBob3JlLndhaXQoY3VycmVudFRpbWUoKSArIHRpbWVvdXQpOwogCiAgICAgLy8g RmluYWxseSwgcG9wIHRoZSBwZW5kaW5nIHN5bmMgcmVwbHkgaW5mb3JtYXRpb24uCiAgICAgewo=