Bug 211127

Summary:

[iOS][WK2] Make sure the network process always resumes its database threads when coming out of suspension

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebKit2

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

achristensen, ggaren, sihui_liu, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Description Chris Dumez 2020-04-28 08:19:01 PDT

Make sure the network process always resumes its database threads when coming out of suspension, even if the main thread is hung.

Comment 1 Chris Dumez 2020-04-28 08:19:15 PDT

<rdar://problem/62463389>

Comment 2 Chris Dumez 2020-04-28 08:27:01 PDT

Created attachment 397839 [details]
Patch

Comment 3 Chris Dumez 2020-04-28 09:02:55 PDT

Created attachment 397843 [details]
Patch

Comment 4 Sihui Liu 2020-04-28 09:06:26 PDT

What are the cases where main thread is blocked by a suspended database thread? Doesn't main thread know database threads' state?

Or we probably should not block main thread on a database thread at all?

Comment 5 Chris Dumez 2020-04-28 09:07:26 PDT

(In reply to Sihui Liu from comment #4)
> What are the cases where main thread is blocked by a suspended database
> thread? Doesn't main thread know database threads' state?
> 
> Or we probably should not block main thread on a database thread at all?

Well, we probably should not but it happens. See rdar://problem/62377357 for one example.

Comment 6 Chris Dumez 2020-04-28 09:10:16 PDT

(In reply to Chris Dumez from comment #5)
> (In reply to Sihui Liu from comment #4)
> > What are the cases where main thread is blocked by a suspended database
> > thread? Doesn't main thread know database threads' state?
> > 
> > Or we probably should not block main thread on a database thread at all?
> 
> Well, we probably should not but it happens. See rdar://problem/62377357 for
> one example.

Alex fixed the case in rdar://problem/62377357 but it is not the first time we have radars about similar issues so I thought we should tweak the design to make sure it does not happen again.

Comment 7 Sihui Liu 2020-04-28 09:56:04 PDT

(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Sihui Liu from comment #4)
> > > What are the cases where main thread is blocked by a suspended database
> > > thread? Doesn't main thread know database threads' state?
> > > 
> > > Or we probably should not block main thread on a database thread at all?
> > 
> > Well, we probably should not but it happens. See rdar://problem/62377357 for
> > one example.
> 
> Alex fixed the case in rdar://problem/62377357 but it is not the first time
> we have radars about similar issues so I thought we should tweak the design
> to make sure it does not happen again.

I see. Looks like we can apply Alex's fix to all the cases where main thread is blocked on a database thread(I currently see ~StorageManagerSet() and waitUntilSyncingLocalStorageFinished()). 

It might be worthwhile to get those fixed and make sure network process not wait especially when it's suspended(m_suspended is true). Your fix will solve most hang issues on suspension, but it covers those blocking on main thread issues and relies on ProcessTaskStateObserver to do the right thing and adds another thread(?).

Comment 8 Chris Dumez 2020-05-07 09:31:46 PDT

We’ve gotten rid of all cases of hangs of the NetworkProcess main thread on database threads. Let’s see if this is sufficient before doing this change, given that it adds complexity.