Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(17)

Issue 2695393003: Make FontListImpl RefCountedThreadSafe (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by tzik
Modified:
2 months, 3 weeks ago
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make FontListImpl RefCountedThreadSafe FontListImpl has non-thread-safe ref count, and this CL replaces it with thread safe ref count. FontListImpl is held by FontList, and |font_cache_| in ResourceBundle holds FontLists. |font_cache_| itself is accessed from multiple thread with a protection of |images_and_fonts_lock_|, but its contents seem exposed without any protection. That implies there is a potential data race if a user of ResourceBundle takes a FontList from GetFontList(), holds a copy of it, and simultaneously touches |font_cache_| by ReloadLocaleResources on another thread. BUG=688072

Patch Set 1 #

Patch Set 2 : +PlatformFont #

Messages

Total messages: 21 (12 generated)
tzik
PTAL
3 months ago (2017-02-16 18:08:29 UTC) #4
sky
I'm sure your fix is correct, but it begs the question of whether FontListImpl is ...
3 months ago (2017-02-16 19:28:10 UTC) #7
Yuki
On 2017/02/16 19:28:10, sky wrote: > I'm sure your fix is correct, but it begs ...
3 months ago (2017-02-17 06:18:27 UTC) #10
tzik
On 2017/02/17 06:18:27, Yuki wrote: > On 2017/02/16 19:28:10, sky wrote: > > I'm sure ...
3 months ago (2017-02-17 13:28:26 UTC) #12
Yuki
On 2017/02/17 13:28:26, tzik wrote: > On 2017/02/17 06:18:27, Yuki wrote: > > On 2017/02/16 ...
3 months ago (2017-02-17 13:36:36 UTC) #14
sky
On Thu, Feb 16, 2017 at 10:18 PM, <yukishiino@chromium.org> wrote: > On 2017/02/16 19:28:10, sky ...
3 months ago (2017-02-17 19:15:43 UTC) #17
Yuki
On 2017/02/17 19:15:43, sky wrote: > On Thu, Feb 16, 2017 at 10:18 PM, <mailto:yukishiino@chromium.org> ...
3 months ago (2017-02-20 03:19:15 UTC) #18
tzik
On 2017/02/20 03:19:15, Yuki wrote: > On 2017/02/17 19:15:43, sky wrote: > > On Thu, ...
3 months ago (2017-02-20 07:34:33 UTC) #20
tzik
3 months ago (2017-02-20 07:40:32 UTC) #21
Fortunately or not, we have only a few number of the problematic cross thread
access to the images and fonts:
https://codereview.chromium.org/2699323002/

Let me try fixing the callers instead of making the ref counts thread safe.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06