Bug 226506 - [webkitcorepy] TaskPool shouldn't fork when 1 process is needed
Summary: [webkitcorepy] TaskPool shouldn't fork when 1 process is needed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-06-01 11:35 PDT by Jonathan Bedard
Modified: 2021-06-02 16:10 PDT (History)
6 users (show)

See Also:


Attachments
Patch (12.65 KB, patch)
2021-06-01 11:38 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (9.37 KB, patch)
2021-06-02 14:42 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2021-06-01 11:35:43 PDT
If we only need a single process, the TaskPool object should not fork. This is important for certain integration tests which use mocking structures that don't function across process boundaries.
Comment 1 Radar WebKit Bug Importer 2021-06-01 11:36:10 PDT
<rdar://problem/78724554>
Comment 2 Jonathan Bedard 2021-06-01 11:38:17 PDT
Created attachment 430279 [details]
Patch
Comment 3 Jonathan Bedard 2021-06-02 14:42:43 PDT
Created attachment 430409 [details]
Patch
Comment 4 dewei_zhu 2021-06-02 15:58:48 PDT
Comment on attachment 430409 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430409&action=review

r=me with comment.

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/task_pool.py:373
>          from mock import patch

Should we ensure we always import this or it's not needed in the case of no-fork?
Comment 5 Jonathan Bedard 2021-06-02 16:01:16 PDT
(In reply to dewei_zhu from comment #4)
> Comment on attachment 430409 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430409&action=review
> 
> r=me with comment.
> 
> > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/task_pool.py:373
> >          from mock import patch
> 
> Should we ensure we always import this or it's not needed in the case of
> no-fork?

Not needed in the case of no-fork.

The reason for including it in the fork case, for those who are curious, is that autoinstalling a package on a child process is racey. This is something we should consider resolving with a file lock (because independently running Python processes have a similar problem), but it hasn't been much of a problem in practice, so we haven't done anything to fix it.
Comment 6 EWS 2021-06-02 16:10:16 PDT
Committed r278380 (238407@main): <https://commits.webkit.org/238407@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430409 [details].