WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
REOPENED
196839
UI↔Web deadlock when printing with a JavaScript alert visible
https://bugs.webkit.org/show_bug.cgi?id=196839
Summary
UI↔Web deadlock when printing with a JavaScript alert visible
Tim Horton
Reported
2019-04-11 17:11:42 PDT
UI↔Web deadlock when printing with a JavaScript alert visible
Attachments
Patch
(14.37 KB, patch)
2019-04-11 17:12 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Tim Horton
Comment 1
2019-04-11 17:12:38 PDT
Created
attachment 367265
[details]
Patch
Tim Horton
Comment 2
2019-04-11 17:12:40 PDT
<
rdar://problem/49157642
>
Andy Estes
Comment 3
2019-04-11 17:44:12 PDT
Comment on
attachment 367265
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367265&action=review
This is all SPI and the only caller of _webViewPrintFormatter doesn't cache the return value, so I think this is ok as-is, but I do think the design of -_webViewPrintFormatter is weird.
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6318 > + if (_page->process().connection()->hasOutstandingOutgoingSynchronousReplies()) > + return nil;
What if a client happens to call this while there are outstanding synchronous replies but doesn't plan to start printing until later? What about always returning a _WKWebViewPrintFormatter but giving it a new boolean property that clients can check?
Alexey Proskuryakov
Comment 4
2019-04-11 20:18:39 PDT
Comment on
attachment 367265
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=367265&action=review
> Source/WebKit/ChangeLog:23 > + synchronous message (like, say, an alert()) results in an app-destroying deadlock.
Is this a regression from switching to delayed reply messages? It used to be that sync messages were handled while waiting for responses.
Tim Horton
Comment 5
2019-04-11 20:34:35 PDT
(In reply to Alexey Proskuryakov from
comment #4
)
> Comment on
attachment 367265
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=367265&action=review
> > > Source/WebKit/ChangeLog:23 > > + synchronous message (like, say, an alert()) results in an app-destroying deadlock. > > Is this a regression from switching to delayed reply messages? It used to be > that sync messages were handled while waiting for responses.
More likely from
http://trac.webkit.org/changeset/236546/webkit
, which has had an endless tail of similar regressions.
Alexey Proskuryakov
Comment 6
2019-04-12 12:04:18 PDT
Indeed.
WebKit Commit Bot
Comment 7
2019-04-17 13:20:46 PDT
Comment on
attachment 367265
[details]
Patch Clearing flags on attachment: 367265 Committed
r244400
: <
https://trac.webkit.org/changeset/244400
>
WebKit Commit Bot
Comment 8
2019-04-17 13:20:47 PDT
All reviewed patches have been landed. Closing bug.
Truitt Savell
Comment 9
2019-04-17 16:06:23 PDT
Reverted
r244400
for reason: Caused testing to exit early with assertionon Debug WK2 Committed
r244407
: <
https://trac.webkit.org/changeset/244407
>
Tim Horton
Comment 10
2019-04-27 14:03:36 PDT
dispatchSyncMessage 1 sendSyncReply 1 dispatchSyncMessage 1 sendSyncReply 1 sendSyncReply 0 sendSyncReply -1 sendSyncReply -2 sendSyncReply -3 sendSyncReply -4 welp
Tim Horton
Comment 11
2019-04-27 23:12:53 PDT
(In reply to Tim Horton from
comment #10
)
> dispatchSyncMessage 1 > sendSyncReply 1 > dispatchSyncMessage 1 > sendSyncReply 1 > sendSyncReply 0 > sendSyncReply -1 > sendSyncReply -2 > sendSyncReply -3 > sendSyncReply -4 > > welp
1 0x10fcde076 IPC::Connection::sendSyncReply(std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >) 2 0x10fdf649d Messages::NetworkProcess::ResetParametersToDefaultValues::send(std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&&, IPC::Connection&) 3 0x10fe452ea auto void IPC::handleMessageAsync<Messages::NetworkProcess::ResetParametersToDefaultValues, WebKit::NetworkProcess, void (WebKit::NetworkProcess::*)(PAL::SessionID, WTF::CompletionHandler<void ()>&&)>(IPC::Connection&, IPC::Decoder&, WebKit::NetworkProcess*, void (WebKit::NetworkProcess::*)(PAL::SessionID, WTF::CompletionHandler<void ()>&&))::'lambda'(auto&&...)::operator()<>(auto&&...) I guess async messages use sendSyncReply too? Confused.
Tim Horton
Comment 12
2019-04-27 23:24:14 PDT
(In reply to Tim Horton from
comment #11
)
> (In reply to Tim Horton from
comment #10
) > > dispatchSyncMessage 1 > > sendSyncReply 1 > > dispatchSyncMessage 1 > > sendSyncReply 1 > > sendSyncReply 0 > > sendSyncReply -1 > > sendSyncReply -2 > > sendSyncReply -3 > > sendSyncReply -4 > > > > welp > > 1 0x10fcde076 > IPC::Connection::sendSyncReply(std::__1::unique_ptr<IPC::Encoder, > std::__1::default_delete<IPC::Encoder> >) > 2 0x10fdf649d > Messages::NetworkProcess::ResetParametersToDefaultValues::send(std::__1:: > unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&&, > IPC::Connection&) > 3 0x10fe452ea auto void > IPC::handleMessageAsync<Messages::NetworkProcess:: > ResetParametersToDefaultValues, WebKit::NetworkProcess, void > (WebKit::NetworkProcess::*)(PAL::SessionID, WTF::CompletionHandler<void > ()>&&)>(IPC::Connection&, IPC::Decoder&, WebKit::NetworkProcess*, void > (WebKit::NetworkProcess::*)(PAL::SessionID, WTF::CompletionHandler<void > ()>&&))::'lambda'(auto&&...)::operator()<>(auto&&...) > > I guess async messages use sendSyncReply too? Confused.
Also this is all happening on a variety of threads, too, so it wasn't safe anyway
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug