Commit graph

6 commits

Author SHA1 Message Date
Steven Moreland
dc43fb279f libutils: wp::fromExisting bugfix
This API was tested before, but it wasn't used until it is needed in
libbinder. Previously it passed the tests because wp::operator==
compares weak_ptrs which are never deleted, but it doesn't check the
value of m_ptr as well. This assumes that the RefBase implementation is
self-consistent.

Future considerations:
- add additional CHECK (perf?)
- add an additional optional CHECK?
- update all refbase tests to use an embellished form of this operator

Bug: 184190315
Test: libutils_test, boot and kill process when libbinder is using this
API

Change-Id: I66c97386d769529d5efae48e06775d4b4a344025
2021-04-07 23:06:09 +00:00
Steven Moreland
da75cef983 ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION
In form, inspired by ANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION.

We get occasional bugs about sp double-ownership. When this flag is
enabled, we have:
- you must construct RefBase objects using sp<>::make
- you must construct wp<> objects by converting them to sp<>
- if you want to convert a raw pointer to an sp<> object (this is
  possible since the refcount is used internally, and is used commonly
  on this*), then you must use 'assertStrongRefExists' semantics which
  aborts if there is no strong ref held. That is, if a client uses
  std::make_shared and then calls a function which internally used to
  call `sp<T>(this)`, you would now call
  `sp<T>::assertStrongRefExists(this)`, and the double ownership
  problem would become a runtime error.

Bug: 184190315
Test: libutils_test
Change-Id: Ie18d3146420df1808e3733027070ec234dda4e9d
2021-04-05 17:45:18 +00:00
Hans Boehm
6e75ad6e13 Revert "Revert "Fix wp and sp comparison bugs""
Fix wp and sp comparison bugs

Make clear() actually clear wp m_refs, so that nulls compare equal.

Make equality consistent with < and >, ensuring that a weak pointer
cannot be both equal to and greater than another.

Don't rely on the built-in < and > operators to correctly order
different objects. The standard does not guarantee that, and there is
a risk of compiler relying on that lack of guarantee.

Remove unnecessary comparison overloads, especially those
comparing a wp<> to an sp<>.

Change the remaining wp<> to sp<> comparisons to check for equivalence
of the mRefs pointer instead of the object address, thus eliminating
the dubious equal comparison result for a dead wp<> and an sp<> that
happen to point to the same object address.

Add comparison tests.

This reverts commit a2a2ad8057.

The original code, and my original CL, both failed to initialize m_refs
in various wp<> constructors. This now became more important, since
comparisons now rely more on m_refs. However I believe it was always
a bug, since some comparisons always relied on m_refs.

Test: Treehugger, boot AOSP, atest RefBase
Bug: 126922090
This reverts commit a2a2ad8057.

Reason for revert: Reapply after constructor fixes.

Change-Id: I2c8917416a2306e36d2b6bb7b397f653020e5688
2019-03-13 13:26:35 -07:00
Hans Boehm
a2a2ad8057 Revert "Fix wp and sp comparison bugs"
This reverts commit 029b12ebde.

Reason for revert: There appear to be problems with null comparisons. Reported failure in HwcBufferCacheTest.

Change-Id: I19745bb281dabe8b05c2df3fe95e7be7a49dcd51
2019-03-13 03:24:12 +00:00
Hans Boehm
029b12ebde Fix wp and sp comparison bugs
Make clear() actually clear wp m_refs, so that nulls compare equal.

Make equality consistent with < and >, ensuring that a weak pointer
cannot be both equal to and greater than another.

Don't rely on the built-in < and > operators to correctly order
different objects. The standard does not guarantee that, and there is
a risk of compiler relying on that lack of guarantee.

Remove unnecessary comparison overloads, especially those
comparing a wp<> to an sp<>.

Change the remaining wp<> to sp<> comparisons to check for equivalence
of the mRefs pointer instead of the object address, thus eliminating
the dubious equal comparison result for a dead wp<> and an sp<> that
happen to point to the same object address.

Add comparison tests.

Test: Treehugger, boot AOSP, atest RefBase
Bug: 126922090
Change-Id: I15911150e0fc85ace2c4b77d337826e12793c690
2019-03-09 21:41:37 -08:00
Elliott Hughes
01705e4f1d Clean up libcutils/libutils tests.
Move tests in the same directory as the corresponding code, so it's
easier to see what is/isn't tested.

Fix naming of libcutils_tests (plural) to match the singular that's more
common (even though the plural makes more sense to me).

Add these two to system/core/'s TEST_MAPPING.

Remove obsolete AndroidTest.xml.

Fix a flaky (timing-dependent) libcutils test.

Test: ran tests
Change-Id: I7e0a31ff45c8a152562bf66fc97161594249366e
2019-02-11 13:15:56 -08:00
Renamed from libutils/tests/RefBase_test.cpp (Browse further)