New issue
Advanced search Search tips

Issue 844268 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-25
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Timeout reduced to 10-14 sec for xmlhttprequests

Reported by shilpa12...@gmail.com, May 18 2018

Issue description

UserAgent: Mozilla/5.0 (Linux; Android 6.0; Lenovo PB2-650M Build/MRA58K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.109 Mobile Safari/537.36

Steps to reproduce the problem:
1. Ajax call from JavaScript to a webservice.
2. Let the response time be >30 sec
3. End up getting a timeout error after 10-14 sec 

What is the expected behavior?
Usual browser request timeout is 5 min and hence ideally shouldn't timeout before that.

What went wrong?
I am not facing this request timed out issue in the current stable version of chrome:66.0.3359.181, I do get the response back (in 40 sec).
It's the same code that I am running on both the versions (stable and beta) of Chrome.

Did this work before? Yes 66.0.3359.181

Does this work in other browsers? Yes

Chrome version: 67.0.3396.40  Channel: n/a
OS Version: 7
Flash Version: 

The code remains the same , nothing has changed except the browser version. Have been testing out how the code works in the beta version and found this.
Encountered this issue on May5, have been trying to get a solution ever since.
Expecting a solution at the earliest, as the timeout error is affecting the code badly.
Thank you.
 
Labels: Needs-Bisect Needs-Triage-M67
Cc: phanindra.mandapaka@chromium.org
Components: Blink>Network>XHR
Labels: Triaged-ET Needs-Feedback
Thanks for filling the issue..

@Reporter:Could you please attach any testfile/jsdfiddle for this with the above test case for ease of repro and checking where this regressed.

Thanks..!
I have attached the HTML file with an AJAX call to a sample web service(.asmx.cs) file, with a time lag of 40 seconds.
The same code when executed in current Chrome version: 66.0.3359.181 results in success, but when executed in version: 67.0.3396.48 results in an error.
I have found the issue to be setting async:"false" in the AJAX call, but do not understand how this has changed between the two versions of Chrome.
If I do not need to execute any two functions asynchronously, then how do I go about doing that with AJAX.
TimeOutService.asmx.cs
825 bytes View Download
htmlTimeOut.html
1.1 KB View Download
Project Member

Comment 4 by sheriffbot@chromium.org, May 18 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Unable to reproduce the issue reported version on 67.0.3396.40 using Win 7 & 10. Attaching Screencast for reference.

Steps 
--------
1. Lauched Chrome.
2. Dowloaded given html file and opened the file.
3. Waited for 5 minutes also opened Dev tools. 
We are unable to see the timeout error after 10-14 sec.

@ Reporter: As we are unable to reproduce the issue from our end. Can you verify this issue with fresh profile that is not having any extensions and apps or reset all the flags.Let us know if we miss anything. 

Thanks!
844268.mp4
1.8 MB View Download

Comment 6 by ricea@chromium.org, May 22 2018

Labels: -Needs-Feedback
I was able to reproduce.

I created a simpler and faster repro. It requires a CGI script that waits 12 seconds to respond.
repro-844268.html
398 bytes View Download
hang-12-seconds.pl
148 bytes View Download

Comment 7 by ricea@chromium.org, May 22 2018

Labels: -Needs-Bisect
Owner: horo@chromium.org
Status: Assigned (was: Unconfirmed)
I performed a bisect. Here's the result:

You are probably looking for a change made after 545753 (known good), but no later than 545754 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/71e4caf1294020283a44cada9aa710fef337c717..5e1b52dd7e828fb2e4bd69f599c0c30eec3e873c


"""
commit 5e1b52dd7e828fb2e4bd69f599c0c30eec3e873c
Author: Tsuyoshi Horo <horo@chromium.org>
Date:   Mon Mar 26 10:24:30 2018 +0000

    Revert "Revert "Use DocumentThreadableLoader for sync loading from worker thread.""
    
    This reverts commit 35cc2b256f2598350915742eefcc505ab902b636 and
    removes "completed_event_->Signal()" from SyncLoadContext::OnReceivedRedirect().
    https://chromium-review.googlesource.com/c/chromium/src/+/657787/23/content/renderer/loader/sync_load_context.cc#81
    
    "completed_event_->Signal()" was called twice when SyncLoadContext receives
    a cross origin redirect response and caused crashes ( crbug.com/797374 ).
    
    Bug:  706331 ,  797374 
[truncated because long]
"""


+horo is this change in timeout behaviour intentional?

Comment 8 by ricea@chromium.org, May 22 2018

#5 I think you need an ASP.NET server to run the sample code from comment #3.

Comment 9 by horo@chromium.org, May 22 2018

Status: Started (was: Assigned)
No. Not intentional.
Created https://crrev.com/c/1068893
Yes you do need ASP.NET server to run the sample code(asmx.cs) file.


Sent by GO Mail

Comment 11 by ricea@chromium.org, May 23 2018

Labels: -Pri-2 Pri-1
I've written a regression test for this issue: https://chromium-review.googlesource.com/c/chromium/src/+/1070089.
Project Member

Comment 12 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74fca1c37eb569a296207ff8b9a36b6a07567a1e

commit 74fca1c37eb569a296207ff8b9a36b6a07567a1e
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Thu May 24 03:13:25 2018

Stop resetting 10 sec timeout in FetchParameters::MakeSynchronous()

Before https://crrev.com/c/657787, FetchParameters::MakeSynchronous() always
calls SetTimeoutInterval(10). So I thought that 10 second is the default timeout
for all synchronous resouce loading.

But this was wrong. The TimeoutInterval of ResourceRequest was not used. After
the CL the value is used for the timeout in SyncLoadContext. But the default
timeout of sync XMLHttpRequest must be infinite. This caused the  bug 844268 .

To fix this bug, this CL stops resetting 10 sec timeout in MakeSynchronous().

Bug:  844268 
Change-Id: I7295ec78637548dc810bb3a8a12a54db4cfbc368
Reviewed-on: https://chromium-review.googlesource.com/1068893
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561374}
[modify] https://crrev.com/74fca1c37eb569a296207ff8b9a36b6a07567a1e/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc

Comment 13 by ricea@chromium.org, May 24 2018

NextAction: 2018-05-25
Verified fixed manually and with my regression test.

It would be good to merge this to M67 stable but I think we are too late.

I will try sending a merge request once it's been in canary a while, anyway.
Project Member

Comment 14 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/044d3d63fb2285585eb7772ca63e76c91aeac3ed

commit 044d3d63fb2285585eb7772ca63e76c91aeac3ed
Author: Adam Rice <ricea@chromium.org>
Date: Thu May 24 06:16:44 2018

Add a test to verify that sync XHR doesn't timeout

Blink had a bug that sync XHR would timeout after 10 seconds. Add a
regression test to make sure the issue does not reoccur.

BUG= 844268 

Change-Id: I161e133582b719102a02349828cd4d9a08773785
Reviewed-on: https://chromium-review.googlesource.com/1070089
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561400}
[add] https://crrev.com/044d3d63fb2285585eb7772ca63e76c91aeac3ed/third_party/WebKit/LayoutTests/external/wpt/xhr/sync-no-timeout.any.js

The NextAction date has arrived: 2018-05-25

Comment 16 by ricea@chromium.org, May 25 2018

Cc: gov...@chromium.org
Labels: Merge-Request-67 M-67 OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac
Requesting merge of #12 to stable. For the people who are affected, it will be a critical issue, as applications that rely on long sync XHR calls will be completely unusable.

Sync XHR is used on roughly 4% of the pages on the web.

The fix is simple and safe.
Project Member

Comment 17 by sheriffbot@chromium.org, May 25 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: We are only 3 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge for CL listed at #12 to M67 branch 3396 based on comment #16. Please merge ASAP so we can pick it up next week stable release. Thank you.
Project Member

Comment 19 by bugdroid1@chromium.org, May 25 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e11497bc51059c4c1b2ebf0cd4537ef8e1d20972

commit e11497bc51059c4c1b2ebf0cd4537ef8e1d20972
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Fri May 25 15:05:32 2018

Merging to M67.

Original commit message:

Stop resetting 10 sec timeout in FetchParameters::MakeSynchronous()

Before https://crrev.com/c/657787, FetchParameters::MakeSynchronous() always
calls SetTimeoutInterval(10). So I thought that 10 second is the default timeout
for all synchronous resouce loading.

But this was wrong. The TimeoutInterval of ResourceRequest was not used. After
the CL the value is used for the timeout in SyncLoadContext. But the default
timeout of sync XMLHttpRequest must be infinite. This caused the  bug 844268 .

To fix this bug, this CL stops resetting 10 sec timeout in MakeSynchronous().

TBR=horo@chromium.org

Bug:  844268 
Change-Id: I7295ec78637548dc810bb3a8a12a54db4cfbc368
Reviewed-on: https://chromium-review.googlesource.com/1068893
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#561374}(cherry picked from commit 74fca1c37eb569a296207ff8b9a36b6a07567a1e)
Reviewed-on: https://chromium-review.googlesource.com/1073527
Cr-Commit-Position: refs/branch-heads/3396@{#701}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/e11497bc51059c4c1b2ebf0cd4537ef8e1d20972/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc

Labels: TE-NeedsTriageFromMTV
We don't have asp.net server set up as per Comment #10  to verify this issue . Hence requesting someone from MTV team to verify this issue on latest M67 stable.

Thanks.! 

Comment 21 by ricea@chromium.org, May 29 2018

Status: Verified (was: Started)
Verified fixed in 67.0.3396.62.
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/843d26289dfee145b3d16c059de4c2494bbb1ac1

commit 843d26289dfee145b3d16c059de4c2494bbb1ac1
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Fri Jun 01 05:11:14 2018

Revert "[Merging to M67] Stop resetting 10 sec timeout in FetchParameters::MakeSynchronous()"

This reverts commit e11497bc51059c4c1b2ebf0cd4537ef8e1d20972.

The original CL was intended to fix the 10 second default timeout issue ( crbug.com/844268 ).
But this CL introduced 100% CPU usage issue by using OneShotTimer with INT_MAX ( crbug.com/848210 ).

So this CL reverts e11497bc51059c4c1b2ebf0cd4537ef8e1d20972, using a large timeout (1 day) to mitigate the timeout issue ( crbug.com/844268 ).

Bug:  848210 ,  844268 
Change-Id: Iccd4a7a9b416e11aca354c658d2e6c9bfc530db4
Reviewed-on: https://chromium-review.googlesource.com/1081777
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#727}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/843d26289dfee145b3d16c059de4c2494bbb1ac1/third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc

Comment 23 by horo@chromium.org, Jun 1 2018

843d26289dfee145b3d16c059de4c2494bbb1ac1 reintroduced the timeout to fix the  issue 848210 .

But the timeout is changed from 10 seconds to 1 day.
So the timeout doesn't cause any issue.


I verified that sync-no-timeout.any.js passes in 67.0.3396.71.

Sign in to add a comment