| Age | Commit message (Collapse) | Author |
|---|
|
Change-Id: I5fc9b14634cd9f2991dc43b2dedb514603d138a1
|
|
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
|
|
Change-Id: I2e1a17105f000737ddb1489ea5c66662226dd2d8
|
|
Bug: 171353386
Test: Builds.
Change-Id: Id8a5cce170682ea43aa641c187c22cb31b6fd017
|
|
This requires a few other changes to support building libutils
properly. It does appear the windows versions of libutils is
referencing CallStack code, but it doesn't seem to cause any problems.
However, I removed those references completely for the windows build.
Also removed a few extra spaces that seem to have accumulated in the
RefBase.cpp.
Bug: 170465278
Test: Builds and libutils unit tests pass.
Change-Id: Ibeee7791b13636b34bdd592c5420fd91620f752a
|
|
Change-Id: I6e221d7a92b6ecb92bb8364757522d4906b86877
|
|
Add FALLTHROUGH_INTENDED for clang compiler.
Bug: 112564944
Test: build with global -Wimplicit-fallthrough.
Change-Id: I40f8bbf94e207c9dd90921e9b762ba51abab5777
|
|
Change-Id: I43a054f155f100b3d7f414e17d3af9b900a05ab5
|
|
This reverts commit b9d0753d2ba88cc60947823e68bb3bed60268361.
Reason for revert: Re-land with MacOS workaround.
Test: Build (on Linux) and boot AOSP, with weak symbols enabled and disabled.
Change-Id: I5150cd90367178f3b039761dca3bccc9c2987df1
|
|
This reverts commit 9d3146af22588e0c23e110be13a515f5347bf687.
Reason for revert: It appears that weak symbols don't work as expected on MacOS, breaking the MacOS aapt build.
Change-Id: Ica0955106485a7bf2e2c3f09ff7910e230eb4139
|
|
Move towards crashing if a normally configured RefBase object is
destroyed without ever incrementing the reference count. We've been
threatening to do this for a long time. The previously last known
violation had been fixed.
This also fixes stack trace printing from RefBase, which had previously
been broken, and which we found necessary to track down further
violations of this rule.
Unfortunately, we found several more violations with the aid of
that fix. After existing CLs are submitted, there are
still some failures, but they are no longer numerous. Thus this CL
doesn't actually crash in the event of a violation, but does log a
verbose stack trace if it encounters one.
Bugs have been filed against the remaining known RefBase client offenders.
We plan to enable crashing on usage violations once those are fixed.
The fix for the stack trace printing breakage unfortunately requires
the use of weak symbols in order to avoid a circular build dependency.
We expect to eventually replace this with execinfo.h functionality.
Some random reformatting, driven by consistency with current formatting
requirements.
Add missing include to BacktraceMap.h.
Bug: 79112958
Bug: 30292291
Test: Boot AOSP, Master
Change-Id: I8151c54560c3b6f75ffc4c48229f0388a2066958
|
|
Conflicts:
init/ueventd.cpp
libion/ion.c
lmkd/lmkd.c
rootdir/init.rc
Change-Id: I05419927e27be1148cd1a2665d51f9a715ab8d47
|
|
Convert fatal log message to non fatal log message, if the weak
reference is decremented to the max count.
Bug: 111479780
CRs-Fixed: 2278739
Change-Id: I7225ee4a2375811fb05053d4c84fc21e30d12391
|
|
Fixes -Wzero-as-null-pointer-constant warning.
Test: m
Bug: 68236239
Change-Id: I5e89ec8c42151875439d2656475a8739ab9cb7dc
|
|
* Move -Wall -Werror from cppflags to cflags.
* Fix/suppress warning on unused variables.
Bug: 66996870
Test: build with WITH_TIDY=1
Change-Id: I1e05e96a1d0bcb2ccef1ce456504b3af57167cc5
|
|
The analyzer is known to be very conservative in the face of atomic
operations (e.g. https://bugs.llvm.org/show_bug.cgi?id=34365); this case
is no different.
It's concerned that `delete this;` might read a different value for
`flags`, and proceed to delete `refs`. Since there are many comments
explaining why this won't happen (and it all looks sane to me), use a
NOLINT to silence this warning.
Analyzer warning:
system/core/libutils/RefBase.cpp:445:5: warning: Use of memory after it
is freed
Bug: 27101951
Test: mma. Use-after-free warning is gone.
Change-Id: Ic1623971bd1bad546fbb12a79439116c89a6762d
|
|
Bug: 36532900
Test: compiled
Change-Id: I3088e1a219e04cf924744d3a0c2d374918bb6395
|
|
moved Foo.h as first include of Foo.cpp, and
removed redundant includes.
Made NativeHandle non virtual.
Test: run & compile
Bug: n/a
Change-Id: I37fa746cd42c9ba23aba181f84cb6c619386406a
|
|
Test: pass
Change-Id: Iec9b9e1e9b6c974124b2043f550fb110cc22846d
|
|
The warnings in these files were hidden by -isystem
framework/native/include.
Bug: 31752268
Test: m -j
Change-Id: I2a54376aea380ee24e6483fb7d35fdfe8991c490
|
|
system/core/include is included in the global include path using
-isystem, which hides all warnings. Fix warnings in libutils
headers in preparation for moving from -isystem to -I.
- Fix implicit cast from int64_t to long in Condition.h. Remove
the __LP64__ check and always compare against LONG_MAX before
casting.
- Fix implicit cast from size_t to ssize_t in KeyedVector.h
- Fix -Wshadow-field-in-constructor warnings in Looper.h and RefBase.h
- Move destructors for MessageHandler and LooperCallback to Looper.cpp
and ReferenceRenamer and VirtualLightRefBase to RefBase.cpp to prevent
vtables in every compilation unit.
- Declare template variables in Singleton.h
- Fix old-style casts in StrongPointer.h and TypeHelpers.h
- Use template metaprogramming in TypeHelpers.h to avoid warnings on
memmove on non-trivial types.
- Add an assignment operator to key_value_pair_t to complete
rule-of-three
- Use memcpy instead of dereferencing a reinterpret_casted pointer to
treat the bits of a float or double as int32_t or int64_t
- Escape unicode sequences inside doxygen comments between \code and
\endcode
- Remove WIN32 ZD definition in Compat.h, %zd works fine with mingw
- Fix WIN32 printf warnings in Filemap.cpp
- Initialize mNullValue with 0 in LruCache.h, some of the tests use a
non-pointer type for TValue.
Test: m -j native
Bug: 31492149
Change-Id: I385a05a3ca01258e44fe3b37ef77e4aaff547b26
|
|
This prevents two different kinds of client errors from causing
undetected memory corruption, and helps with the detection of others:
1. We no longer deallocate objects when the weak count goes to zero
and there have been no strong references. This otherwise causes
us to return a garbage object from a constructor if the constructor
allocates and deallocates a weak pointer to this. And we do know
that clients allocate such weak pointers in constructors and their
lifetime is hard to trace.
2. We abort if a RefBase object is explicitly destroyed while
the weak count is nonzero. Otherwise a subsequent decrement
would cause a write to potentially reallocated memory.
3. We check counter values returned by atomic decrements for
plausibility, and fail immediately if they are not plausible.
We unconditionally log any cases in which 1 changes behavior
from before. We abort in cases in which 2 changes behavior, since
those reflect clear bugs.
In case 1, a log message now indicates a possible leak. We have
not seen such a message in practice.
The third point introduces a small amount of overhead into the
reference count decrement path. But this should be negligible
compared to the actual decrement cost.
Add a test for promote/attemptIncStrong that tries to check for
both (1) above and concurrent operation of attemptIncStrong.
Add some additional warnings and explanations to the RefBase
documentation.
Bug: 30503444
Bug: 30292291
Bug: 30292538
Change-Id: Ida92b9a2e247f543a948a75d221fbc0038dea66c
|
|
Add basic interface documentation to RefBase.h.
Much, but not all, of this is cut-and-pasted from an email message
from Mathias Agopian. The rest is reconstructed from the code.
Delete some, now redundant, text from Refbase.cpp, and add a bit
more about the implementation strategy.
Some minor fixes to internal comments.
Bug: 30292291
Change-Id: I56518ae5553bc6de0cc2331778e7fcf2e6c4fd87
|
|
The compensating onLastStrongRef call could be made even when there
was no onIncStrongAttempted call to compensate for. This
happened in the OBJECT_LIFETIME_STRONG case when e.g. curCount
was initially zero, but was concurrently incremented by another
thread.
I believe the old code was also incorrect in the
curCount = INITIAL_STRONG_VALUE + 1 case,
which seems to be possible under unlikely conditions.
In that case, I believe the compensating call IS needed.
Thus the condition was also changed.
Bug: 30503444
Change-Id: I44bcbcbb1264e4b52b6d3750dc39b041c4140381
|
|
Convert to use std::atomic directly.
Consistently use relaxed ordering for increments, release ordering
for decrements, and an added acquire fence when the count goes to
zero.
Fix what looks like another race in attemptIncStrong:
It seems entirely possible that the final adjustment for
INITIAL_STRONG_VALUE would see e.g. INITIAL_STRONG_VALUE + 1,
since we could be running in the middle of another initial
increment.
Attempt to somewhat document what this actually does, and
what's expected from the client. Hide the documentation in
the .cpp file for now.
Remove a confusing redundant test in decWeak. OBJECT_LIFETIME_STRONG
and OBJECT_LIFETIME_WEAK are the only options, in spite of some
of the original comments.
It's conceivable that either of these issues has resulted in
actual crashes, though I would guess the probability is small.
It's hard enough to reason about this code without the bugs.
Bug: 28705989
Change-Id: I4107a56c3fc0fdb7ee17fc8a8f0dd7fb128af9d8
|
|
Bug: 28341362
Change-Id: I4504e98a8db31e0edcbe63c23f9af43eb13e9d86
|
|
Also cleans up two instances of open() with useless mode params, and
changes a few uses of snprintf to use sizeof(buffer) instead of
hardcoded buffer sizes.
Change-Id: If11591003d910c995e72ad8f75afd072c255a3c5
|
|
- Deal with some -Wunused issues
- Override PRI macros (windows)
- Revert use of PRI macros on off64_t (linux)
- Deal with a gnu++11 complaince issue
Change-Id: Ie66751293bd84477a5a6dfd8a57e700a16e36964
|
|
This was broken about 5 months ago in change I78435ed49aa196a0efb45bf9b2d58b62c41737d3.
See: https://goto.google.com/jhtss
Change-Id: Icc32993552efed3015bc1b79a7bd872d7510e020
|
|
Change-Id: I6cf4268599460791414882f91eeb88a992fbd29d
|
