Making CSSValue Pool thread local

Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static
CSSValuePool instances per thread when needed, and the
cleanup code is handled in ThreadState::cleanup() added by
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the
ThreadState::cleanup() as the worker thread) need to be
modified so that false positive leak errors will not be
reported.

In addition, an indirect memory leak "__strdup
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is
generated in webkit unit tests; but after printing out the
full error stack trace, we observe that it eventually
originates from libfontconfig, a third_party library that
has leaks and has already been suppressed in
leak_suppression.cc. But the default stack trace is too
short on suppress this indirect memory leak; so we added
one more leak suppression underneath the libfontconfig.

BUG=599659
Committed: https://crrev.com/1174eb6ddb160985b51d1d45321e24c493aa7f83
Cr-Commit-Position: refs/heads/master@{#388815}

[xlai (Olivia), 2016-04-06 22:23:17]

Patchset #1 (id:1) has been deleted

[esprehn, 2016-04-06 23:23:59]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org, haraken@chromium.org

[esprehn, 2016-04-06 23:23:59]

haraken@ This seems like it should work. I also think we should change
DEFINE_STATIC_LOCAL to not magic up a Persistent for GC types, that was really
confusing here before.

[haraken, 2016-04-07 02:37:34]

haraken@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org

[haraken, 2016-04-07 02:37:35]

On 2016/04/06 23:23:59, esprehn wrote:
> haraken@ This seems like it should work. I also think we should change
> DEFINE_STATIC_LOCAL to not magic up a Persistent for GC types, that was really
> confusing here before.

Currently this does not work because:

1) Oilpan does not yet support DEFINE_STATIC_LOCAL(Persistent, ...) called by
non-main threads.

2) Oilpan forces a restriction that all persistent handles must be gone before
the thread shuts down (to make sure that all destructors are called before the
shutdown and thus there is no memory leak). This rule is problematic for you
because you want to intentionally leak the CSSValuePool.

It's possible to fix the Oilpan's rule somehow, but I don't think
https://codereview.chromium.org/1863793003/ is a bad idea.
https://codereview.chromium.org/1863793003/ is messy because you distinguish
main-thread's CSSValuePool (defined in DEFINE_STATIC_LOCAL) from
non-main-thread's CSSValuePool (defined in ExecutionContext). If you make the
ExecutionContext always (= main-thread + non-main-thread) hold the CSSValuePool,
can you clean up the CL more?

[esprehn, 2016-04-07 03:05:52]

What's the timeline on fixing oilpan to support thread local statics? I'm
worried we're going to hit this more and more.

[haraken, 2016-04-07 04:27:55]

On 2016/04/07 03:05:52, esprehn wrote:
> What's the timeline on fixing oilpan to support thread local statics? I'm
> worried we're going to hit this more and more.

I'm not sure if we really want to support it, because static persistent handles
created in non-main threads have a risk of producing leaks. If we store the
persistent handles on an ExecutionContext, they (+ all objects retained by the
persistent handles) are guaranteed to get destructed at the end of the thread
shutdown. Which seems better to me.

[esprehn, 2016-04-07 04:34:42]

On 2016/04/07 at 04:27:55, haraken wrote:
> On 2016/04/07 03:05:52, esprehn wrote:
> > What's the timeline on fixing oilpan to support thread local statics? I'm
> > worried we're going to hit this more and more.
> 
> I'm not sure if we really want to support it, because static persistent
handles created in non-main threads have a risk of producing leaks. If we store
the persistent handles on an ExecutionContext, they (+ all objects retained by
the persistent handles) are guaranteed to get destructed at the end of the
thread shutdown. Which seems better to me.

That requires ExecutuionContext or the subclass to learn about all the features
and for each component to pass the cache down through every layer. We should
just be able to write with TLS like we could before oilpan. A static local
ThreadSpecific to a Persistent should work so your module can be encapsulated.

We could add some kind of ThreadSpecific that tacks data onto a list in
WTFThreadData I guess? Thread shutdown can clear all the handles to prevent
leaks?

[haraken, 2016-04-07 04:41:55]

On 2016/04/07 04:34:42, esprehn wrote:
> On 2016/04/07 at 04:27:55, haraken wrote:
> > On 2016/04/07 03:05:52, esprehn wrote:
> > > What's the timeline on fixing oilpan to support thread local statics? I'm
> > > worried we're going to hit this more and more.
> > 
> > I'm not sure if we really want to support it, because static persistent
> handles created in non-main threads have a risk of producing leaks. If we
store
> the persistent handles on an ExecutionContext, they (+ all objects retained by
> the persistent handles) are guaranteed to get destructed at the end of the
> thread shutdown. Which seems better to me.
> 
> That requires ExecutuionContext or the subclass to learn about all the
features
> and for each component to pass the cache down through every layer. We should
> just be able to write with TLS like we could before oilpan. A static local
> ThreadSpecific to a Persistent should work so your module can be encapsulated.

We already have a ton of DEFINE_STATIC_LOCALs that stores static persistents,
but none of them is used by non-main threads. CSSValuePool is the first
exception.

> We could add some kind of ThreadSpecific that tacks data onto a list in
> WTFThreadData I guess? Thread shutdown can clear all the handles to prevent
> leaks?

Yes, technically we can do this.

[xlai (Olivia), 2016-04-07 15:19:40]

On 2016/04/07 04:41:55, haraken wrote:
> On 2016/04/07 04:34:42, esprehn wrote:
> > On 2016/04/07 at 04:27:55, haraken wrote:
> > > On 2016/04/07 03:05:52, esprehn wrote:
> > > > What's the timeline on fixing oilpan to support thread local statics?
I'm
> > > > worried we're going to hit this more and more.
> > > 
> > > I'm not sure if we really want to support it, because static persistent
> > handles created in non-main threads have a risk of producing leaks. If we
> store
> > the persistent handles on an ExecutionContext, they (+ all objects retained
by
> > the persistent handles) are guaranteed to get destructed at the end of the
> > thread shutdown. Which seems better to me.
> > 
> > That requires ExecutuionContext or the subclass to learn about all the
> features
> > and for each component to pass the cache down through every layer. We should
> > just be able to write with TLS like we could before oilpan. A static local
> > ThreadSpecific to a Persistent should work so your module can be
encapsulated.
> 
> We already have a ton of DEFINE_STATIC_LOCALs that stores static persistents,
> but none of them is used by non-main threads. CSSValuePool is the first
> exception.
> 
> > We could add some kind of ThreadSpecific that tacks data onto a list in
> > WTFThreadData I guess? Thread shutdown can clear all the handles to prevent
> > leaks?
> 
> Yes, technically we can do this.

Is the idea of tacking data onto a list in WTFThreadData going to be a big
implementation?
I'm hoping to make use CSS caches for worker threads within this quarter so I
hope to know the timeline.

Otherwise, as haraken@ suggested, https://codereview.chromium.org/1863793003/
isn't a bad idea. 
However, the reason why I distinguish the main-thread CSS caches and
non-main-thread CSS caches is that 
I tried not to affect the existing code paths as much as possible. 
Currently, there're 544 places that references the main-thread CSSValuePool
static instance. 
It's going to be a big refactoring if I clean up that patch and make
CSSValuePool only stay in ExecutionContext.
That really sounds like an overkill.

[sof, 2016-04-07 15:38:05]

On 2016/04/07 04:41:55, haraken wrote:
> On 2016/04/07 04:34:42, esprehn wrote:
> > On 2016/04/07 at 04:27:55, haraken wrote:
> > > On 2016/04/07 03:05:52, esprehn wrote:
> > > > What's the timeline on fixing oilpan to support thread local statics?
I'm
> > > > worried we're going to hit this more and more.
> > > 
> > > I'm not sure if we really want to support it, because static persistent
> > handles created in non-main threads have a risk of producing leaks. If we
> store
> > the persistent handles on an ExecutionContext, they (+ all objects retained
by
> > the persistent handles) are guaranteed to get destructed at the end of the
> > thread shutdown. Which seems better to me.
> > 
> > That requires ExecutuionContext or the subclass to learn about all the
> features
> > and for each component to pass the cache down through every layer. We should
> > just be able to write with TLS like we could before oilpan. A static local
> > ThreadSpecific to a Persistent should work so your module can be
encapsulated.
> 
> We already have a ton of DEFINE_STATIC_LOCALs that stores static persistents,
> but none of them is used by non-main threads. CSSValuePool is the first
> exception.
> 

We had one not so long ago, phased out by
https://codereview.chromium.org/1667813002

[haraken, 2016-04-08 01:29:14]

On 2016/04/07 15:19:40, xlai (Olivia) wrote:
> On 2016/04/07 04:41:55, haraken wrote:
> > On 2016/04/07 04:34:42, esprehn wrote:
> > > On 2016/04/07 at 04:27:55, haraken wrote:
> > > > On 2016/04/07 03:05:52, esprehn wrote:
> > > > > What's the timeline on fixing oilpan to support thread local statics?
> I'm
> > > > > worried we're going to hit this more and more.
> > > > 
> > > > I'm not sure if we really want to support it, because static persistent
> > > handles created in non-main threads have a risk of producing leaks. If we
> > store
> > > the persistent handles on an ExecutionContext, they (+ all objects
retained
> by
> > > the persistent handles) are guaranteed to get destructed at the end of the
> > > thread shutdown. Which seems better to me.
> > > 
> > > That requires ExecutuionContext or the subclass to learn about all the
> > features
> > > and for each component to pass the cache down through every layer. We
should
> > > just be able to write with TLS like we could before oilpan. A static local
> > > ThreadSpecific to a Persistent should work so your module can be
> encapsulated.
> > 
> > We already have a ton of DEFINE_STATIC_LOCALs that stores static
persistents,
> > but none of them is used by non-main threads. CSSValuePool is the first
> > exception.
> > 
> > > We could add some kind of ThreadSpecific that tacks data onto a list in
> > > WTFThreadData I guess? Thread shutdown can clear all the handles to
prevent
> > > leaks?
> > 
> > Yes, technically we can do this.
> 
> Is the idea of tacking data onto a list in WTFThreadData going to be a big
> implementation?
> I'm hoping to make use CSS caches for worker threads within this quarter so I
> hope to know the timeline.
> 
> Otherwise, as haraken@ suggested, https://codereview.chromium.org/1863793003/
> isn't a bad idea. 
> However, the reason why I distinguish the main-thread CSS caches and
> non-main-thread CSS caches is that 
> I tried not to affect the existing code paths as much as possible. 
> Currently, there're 544 places that references the main-thread CSSValuePool
> static instance. 
> It's going to be a big refactoring if I clean up that patch and make
> CSSValuePool only stay in ExecutionContext.
> That really sounds like an overkill.

Fair enough. It looks better to implement the idea of tacking data onto a list
in WTFThreadData.

(I won't have a bandwidth to work on it soon but am happy to review CLs.)

[xlai (Olivia), 2016-04-18 15:09:34]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-18 15:09:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870503002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870503002/40001

[commit-bot: I haz the power, 2016-04-18 16:47:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-18 16:47:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xlai (Olivia), 2016-04-18 18:42:37]

After jbroman@'s patch (https://codereview.chromium.org/1881933005/) that cleans
up static thread-specific persistent has been landed, I continue trying to make
CSSValuePool thread local. 

I would like to seek advice on handling with the memory leak in StringImpl, as
seen in the two failed unit tests in ASAN.
@esprehn: StringImpl is not garbage collected, shouldn't
blink::XMLDocumentParser::end() then cleans up all of them when finish parsing?

[xlai (Olivia), 2016-04-18 22:17:44]

Alright, this has something to do with the webkit unit tests: the ThreadState is
not attached/detached with the thread that's launching the unit tests. As a
result, the cleanup code that carefully handles all the persistent handles is
not invoked at all in unit tests.

[haraken, 2016-04-19 01:00:48]

The change LGTM, once you fix the unit tests (before landing this CL).

[xlai (Olivia), 2016-04-20 00:29:41]

Description was changed from

==========
Making CSSValue Pool thread local

BUG=599659
==========

to

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread only. But
OffscreenCanvas in a worker requires to access the CSS value caches in a
non-main thread. This patch uses the ThreadSpecific persistent handles to create
static CSSValuePool instances per thread when needed, and the cleanup code is
handled in ThreadState::cleanup() added by patch
https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the ThreadState::cleanup() as
the worker thread) need to be modified so that false positive leak errors will
not be reported.

In addition, an indirect memory leak "__strdup
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is generated in webkit unit
tests; but after printing out the full error stack trace, we observe that it
eventually originates from libfontconfig, a third_party library that has leaks
and has
already been suppressed in leak_suppression.cc. But the default stack trace is
too short on suppress this indirect memory leak; so we added one more leak
suppression underneath the libfontconfig.

BUG=599659
==========

[xlai (Olivia), 2016-04-20 13:39:32]

Description was changed from

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread only. But
OffscreenCanvas in a worker requires to access the CSS value caches in a
non-main thread. This patch uses the ThreadSpecific persistent handles to create
static CSSValuePool instances per thread when needed, and the cleanup code is
handled in ThreadState::cleanup() added by patch
https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the ThreadState::cleanup() as
the worker thread) need to be modified so that false positive leak errors will
not be reported.

In addition, an indirect memory leak "__strdup
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is generated in webkit unit
tests; but after printing out the full error stack trace, we observe that it
eventually originates from libfontconfig, a third_party library that has leaks
and has
already been suppressed in leak_suppression.cc. But the default stack trace is
too short on suppress this indirect memory leak; so we added one more leak
suppression underneath the libfontconfig.

BUG=599659
==========

to

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access 
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static 
CSSValuePool instances per thread when needed, and the 
cleanup code is handled in ThreadState::cleanup() added by 
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the 
ThreadState::cleanup() as the worker thread) need to be 
modified so that false positive leak errors will not be 
reported.

In addition, an indirect memory leak "__strdup 
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is 
generated in webkit unit tests; but after printing out the 
full error stack trace, we observe that it eventually 
originates from libfontconfig, a third_party library that 
has leaks and has already been suppressed in 
leak_suppression.cc. But the default stack trace is too 
short on suppress this indirect memory leak; so we added 
one more leak suppression underneath the libfontconfig.


BUG=599659
==========

[xlai (Olivia), 2016-04-20 13:53:10]

xlai@chromium.org changed reviewers:
+ eugenis@chromium.org

[xlai (Olivia), 2016-04-20 13:53:11]

Adding eugenis@ to review the sanitizers/

eugenis@: Right now, the webkit unit tests generate false positive Indirect
Leaks in __strdup /build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c:42; but when
I unwind the stack trace using "fast_unwind_on_malloc" flag, I can see that the
third method in the stack trace comes from libfontconfig, which is a third-party
library that causes leaks and has been already suppressed in
lsan_suppression.cc. But by default only two methods in the stack trace are
unwound in LSAN/ASAN so the current suppression doesn't work in this case.

[Evgeniy Stepanov, 2016-04-20 20:53:25]

eugenis@google.com changed reviewers:
+ eugenis@google.com

[Evgeniy Stepanov, 2016-04-20 20:53:26]

https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
File build/sanitizers/lsan_suppressions.cc (right):

https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
build/sanitizers/lsan_suppressions.cc:28: "leak:eglibc-2.19/string\n"
This would break on any different version of glibc.
Also, this effectively disables all leak detection for memory allocated in
strdup, which we would really like to avoid.

Do you have the full stack trace (with fast_unwind_on_malloc=0)?

How do I reproduce it? Would running webkit_unit_tests with asan+lsan do it?

[Evgeniy Stepanov, 2016-04-20 21:13:07]

https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
File build/sanitizers/lsan_suppressions.cc (right):

https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
build/sanitizers/lsan_suppressions.cc:28: "leak:eglibc-2.19/string\n"
On 2016/04/20 20:53:26, Evgeniy Stepanov wrote:
> This would break on any different version of glibc.
> Also, this effectively disables all leak detection for memory allocated in
> strdup, which we would really like to avoid.
> 
> Do you have the full stack trace (with fast_unwind_on_malloc=0)?
> 
> How do I reproduce it? Would running webkit_unit_tests with asan+lsan do it?

OK, I've got it. I'll fix ASan/LSan upstream so that this stacktrace would look
like
1. strdup
2. /path/to/libfontconfig.so.1

Then the existing suppression would apply.

In the mean time, please change this suppression to "leak:__strdup" and file a
bug to remove it. Make this bug blocked by crbug.com/604993.

[xlai (Olivia), 2016-04-20 21:41:53]

On 2016/04/20 21:13:07, Evgeniy Stepanov wrote:
>
https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
> File build/sanitizers/lsan_suppressions.cc (right):
> 
>
https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
> build/sanitizers/lsan_suppressions.cc:28: "leak:eglibc-2.19/string\n"
> On 2016/04/20 20:53:26, Evgeniy Stepanov wrote:
> > This would break on any different version of glibc.
> > Also, this effectively disables all leak detection for memory allocated in
> > strdup, which we would really like to avoid.
> > 
> > Do you have the full stack trace (with fast_unwind_on_malloc=0)?
> > 
> > How do I reproduce it? Would running webkit_unit_tests with asan+lsan do it?
> 
> OK, I've got it. I'll fix ASan/LSan upstream so that this stacktrace would
look
> like
> 1. strdup
> 2. /path/to/libfontconfig.so.1
> 
> Then the existing suppression would apply.
> 
> In the mean time, please change this suppression to "leak:__strdup" and file a
> bug to remove it. Make this bug blocked by crbug.com/604993.

Changed the suppression to "leak:__strdup".

Also I created a bug (http://crbug.com/605286) and assigned it to you; I put the
long
stack trace of the indirect leak there.

[Evgeniy Stepanov, 2016-04-20 21:42:51]

On 2016/04/20 21:41:53, xlai (Olivia) wrote:
> On 2016/04/20 21:13:07, Evgeniy Stepanov wrote:
> >
>
https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
> > File build/sanitizers/lsan_suppressions.cc (right):
> > 
> >
>
https://codereview.chromium.org/1870503002/diff/60001/build/sanitizers/lsan_s...
> > build/sanitizers/lsan_suppressions.cc:28: "leak:eglibc-2.19/string\n"
> > On 2016/04/20 20:53:26, Evgeniy Stepanov wrote:
> > > This would break on any different version of glibc.
> > > Also, this effectively disables all leak detection for memory allocated in
> > > strdup, which we would really like to avoid.
> > > 
> > > Do you have the full stack trace (with fast_unwind_on_malloc=0)?
> > > 
> > > How do I reproduce it? Would running webkit_unit_tests with asan+lsan do
it?
> > 
> > OK, I've got it. I'll fix ASan/LSan upstream so that this stacktrace would
> look
> > like
> > 1. strdup
> > 2. /path/to/libfontconfig.so.1
> > 
> > Then the existing suppression would apply.
> > 
> > In the mean time, please change this suppression to "leak:__strdup" and file
a
> > bug to remove it. Make this bug blocked by crbug.com/604993.
> 
> Changed the suppression to "leak:__strdup".
> 
> Also I created a bug (http://crbug.com/605286) and assigned it to you; I put
the
> long
> stack trace of the indirect leak there.

Thanks. LSan suppression LGTM.

[xlai (Olivia), 2016-04-21 14:26:59]

The CQ bit was checked by xlai@chromium.org

[xlai (Olivia), 2016-04-21 14:26:59]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1870503002/#ps80001
(title: "Change based on eugenis's feedback")

[commit-bot: I haz the power, 2016-04-21 14:27:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870503002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870503002/80001

[commit-bot: I haz the power, 2016-04-21 14:45:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-21 14:45:42]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xlai (Olivia), 2016-04-21 14:52:48]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2016-04-21 14:53:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870503002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870503002/80001

[commit-bot: I haz the power, 2016-04-21 15:13:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-21 15:13:27]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xlai (Olivia), 2016-04-21 16:48:29]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2016-04-21 16:48:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870503002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870503002/80001

[commit-bot: I haz the power, 2016-04-21 18:16:25]

Description was changed from

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access 
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static 
CSSValuePool instances per thread when needed, and the 
cleanup code is handled in ThreadState::cleanup() added by 
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the 
ThreadState::cleanup() as the worker thread) need to be 
modified so that false positive leak errors will not be 
reported.

In addition, an indirect memory leak "__strdup 
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is 
generated in webkit unit tests; but after printing out the 
full error stack trace, we observe that it eventually 
originates from libfontconfig, a third_party library that 
has leaks and has already been suppressed in 
leak_suppression.cc. But the default stack trace is too 
short on suppress this indirect memory leak; so we added 
one more leak suppression underneath the libfontconfig.


BUG=599659
==========

to

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access 
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static 
CSSValuePool instances per thread when needed, and the 
cleanup code is handled in ThreadState::cleanup() added by 
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the 
ThreadState::cleanup() as the worker thread) need to be 
modified so that false positive leak errors will not be 
reported.

In addition, an indirect memory leak "__strdup 
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is 
generated in webkit unit tests; but after printing out the 
full error stack trace, we observe that it eventually 
originates from libfontconfig, a third_party library that 
has leaks and has already been suppressed in 
leak_suppression.cc. But the default stack trace is too 
short on suppress this indirect memory leak; so we added 
one more leak suppression underneath the libfontconfig.


BUG=599659
==========

[commit-bot: I haz the power, 2016-04-21 18:16:26]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:36:10]

Description was changed from

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access 
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static 
CSSValuePool instances per thread when needed, and the 
cleanup code is handled in ThreadState::cleanup() added by 
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the 
ThreadState::cleanup() as the worker thread) need to be 
modified so that false positive leak errors will not be 
reported.

In addition, an indirect memory leak "__strdup 
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is 
generated in webkit unit tests; but after printing out the 
full error stack trace, we observe that it eventually 
originates from libfontconfig, a third_party library that 
has leaks and has already been suppressed in 
leak_suppression.cc. But the default stack trace is too 
short on suppress this indirect memory leak; so we added 
one more leak suppression underneath the libfontconfig.


BUG=599659
==========

to

==========
Making CSSValue Pool thread local

CSSValuePool was used to be static instance on main thread
only. But OffscreenCanvas in a worker requires to access
the CSS value caches in a non-main thread. This patch uses
 the ThreadSpecific persistent handles to create static
CSSValuePool instances per thread when needed, and the
cleanup code is handled in ThreadState::cleanup() added by
patch https://codereview.chromium.org/1881933005.

As a result, WebKit unit tests (which does not use the
ThreadState::cleanup() as the worker thread) need to be
modified so that false positive leak errors will not be
reported.

In addition, an indirect memory leak "__strdup
/build/eglibc-3GlaMS/eglibc-2.19/string/strdup.c" is
generated in webkit unit tests; but after printing out the
full error stack trace, we observe that it eventually
originates from libfontconfig, a third_party library that
has leaks and has already been suppressed in
leak_suppression.cc. But the default stack trace is too
short on suppress this indirect memory leak; so we added
one more leak suppression underneath the libfontconfig.

BUG=599659

Committed: https://crrev.com/1174eb6ddb160985b51d1d45321e24c493aa7f83
Cr-Commit-Position: refs/heads/master@{#388815}
==========

[commit-bot: I haz the power, 2016-04-22 19:36:12]

Patchset 4 (id:??) landed as
https://crrev.com/1174eb6ddb160985b51d1d45321e24c493aa7f83
Cr-Commit-Position: refs/heads/master@{#388815}