Leaky seems like the right default.
See also https://groups.google.com/a/chromium.org/d/msg/chromium-dev/OLS4JSZvowI/-qUz9xzJBAAJ .
This is also related to https://crbug.com/686866 as only Leaky's can be switched to a simple static initializer.
I was thinking first I'd rename DefaultLazyInstanceTraits to DestructorOnExitLazyInstanceTraits (or something) and then require the selection of which behaviour is desired, either LazyInstance<T>::Leaky, or LazyInstance<T>::DestructorOnExit explicitly. From a quick look, I believe many instances of non-Leaky are simply because the author didn't think about it, and so ended up with default non-Leaky behaviour.
This would be a slightly large change because of https://cs.chromium.org/search/?q=friend.*defaultlazyinstancetraits+package:%5Echromium$&type=cs .
But, having done that, I think it'll be easier to reduce the number of non-Leaky by looking at each one individually.
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6
commit 1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6
Author: Daniel Cheng <dcheng@chromium.org>
Date: Tue Jan 23 23:51:31 2018
Make DestructorAtExit LazyInstances leak.
This means that tests may no longer rely on LazyInstance<T> to clean
up global state between tests. Tests that depend on globals but need
a clean test environment should use explicit test hooks to reset this
state.
Only one test depends on DestructorAtExit for correct functionality.
cast_audio_backend_unittests creates several mocks that are owned by a
base::LazyInstance. Since Gmock verifies expectations in destructors,
switching all LazyInstances to leaky ones breaks this verification.
The solution is to manually verify and check the expectations at the
end of the test.
Note that while the mock objects are annotated as leaked for Gmock,
that does not mean the cast_audio_backend_unittests will leak
arbitrary amounts of memory. The test fixture already configures
StreamMixer for each test run; this setup will clear up any leftover
objects from the previous test.
DestructorAtExit will be removed in a followup.
Bug: 698982
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I822ce507dbb98067a788466e7c8fcc96c3a64ef9
Reviewed-on: https://chromium-review.googlesource.com/874994
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531385}
[modify] https://crrev.com/1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6/base/lazy_instance.h
[modify] https://crrev.com/1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6/base/lazy_instance_unittest.cc
[modify] https://crrev.com/1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6/chromecast/media/cma/backend/alsa/stream_mixer_unittest.cc
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/0d4683df865710d6eedbef9e6bd75c401098fc25
commit 0d4683df865710d6eedbef9e6bd75c401098fc25
Author: Joe Downing <joedow@chromium.org>
Date: Wed Jan 24 01:32:38 2018
Revert "Make DestructorAtExit LazyInstances leak."
This reverts commit 1b97e93c5a1d0bc0ed2f84d0d34581eade8315c6.
Reason for revert: Speculative revert due to Linux TSAN unittest errors:
https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20TSan%20Tests/builds/17057
Original change's description:
> Make DestructorAtExit LazyInstances leak.
>
> This means that tests may no longer rely on LazyInstance<T> to clean
> up global state between tests. Tests that depend on globals but need
> a clean test environment should use explicit test hooks to reset this
> state.
>
> Only one test depends on DestructorAtExit for correct functionality.
> cast_audio_backend_unittests creates several mocks that are owned by a
> base::LazyInstance. Since Gmock verifies expectations in destructors,
> switching all LazyInstances to leaky ones breaks this verification.
> The solution is to manually verify and check the expectations at the
> end of the test.
>
> Note that while the mock objects are annotated as leaked for Gmock,
> that does not mean the cast_audio_backend_unittests will leak
> arbitrary amounts of memory. The test fixture already configures
> StreamMixer for each test run; this setup will clear up any leftover
> objects from the previous test.
>
> DestructorAtExit will be removed in a followup.
>
> Bug: 698982
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I822ce507dbb98067a788466e7c8fcc96c3a64ef9
> Reviewed-on: https://chromium-review.googlesource.com/874994
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531385}
TBR=dcheng@chromium.org,gab@chromium.org,kmackay@chromium.org
Change-Id: Ief1f9169caa0ff81e2c095df0a0520a71d7640a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 698982
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/882506
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531398}
[modify] https://crrev.com/0d4683df865710d6eedbef9e6bd75c401098fc25/base/lazy_instance.h
[modify] https://crrev.com/0d4683df865710d6eedbef9e6bd75c401098fc25/base/lazy_instance_unittest.cc
[modify] https://crrev.com/0d4683df865710d6eedbef9e6bd75c401098fc25/chromecast/media/cma/backend/alsa/stream_mixer_unittest.cc
Comment 1 by scottmg@chromium.org
, Mar 8 2017