Bug 119767 - [WK2] Sync messages with non-default timeout sent from secondary threads always time out
Summary: [WK2] Sync messages with non-default timeout sent from secondary threads alwa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.8
: P2 Major
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-13 13:49 PDT by Justin Bur
Modified: 2013-08-14 14:35 PDT (History)
1 user (show)

See Also:


Attachments
proposed patch (494 bytes, patch)
2013-08-13 13:49 PDT, Justin Bur
no flags Details | Formatted Diff | Diff
patch with ChangeLog (1.33 KB, patch)
2013-08-13 14:24 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Bur 2013-08-13 13:49:44 PDT
Created attachment 208670 [details]
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.
Comment 1 Alexey Proskuryakov 2013-08-13 14:17:42 PDT
<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?
Comment 2 Alexey Proskuryakov 2013-08-13 14:24:23 PDT
Created attachment 208676 [details]
patch with ChangeLog
Comment 3 Justin Bur 2013-08-13 15:54:06 PDT
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.
Comment 4 Alexey Proskuryakov 2013-08-13 17:05:31 PDT
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.
Comment 5 Justin Bur 2013-08-13 17:27:16 PDT
(In reply to comment #4)
Ah, that is good to know. I will make my code comply with that restriction. Thank you!
Comment 6 WebKit Commit Bot 2013-08-14 14:35:46 PDT
Comment on attachment 208676 [details]
patch with ChangeLog

Clearing flags on attachment: 208676

Committed r154073: <http://trac.webkit.org/changeset/154073>
Comment 7 WebKit Commit Bot 2013-08-14 14:35:48 PDT
All reviewed patches have been landed.  Closing bug.