We're using two different functions to atomically decrement a reference count,
the native `atomic_fetch_sub` and the managed `Interlocked.Decrement`.
Unfortunately the return value is not the same: `atomic_fetch_sub` returns the
original value before the subtraction, while `Interlocked.Decrement` returns
the subtracted value, while our code assumed the functions behaved the same.
This resulted in a memory leak, because we'd incorrectly expect `0` to be
returned from `atomic_fetch_sub` when the reference count reaches zero, and
thus not detect when the descriptor a block should be freed.
The fix is to update the expected return value from `atomic_fetch_sub` to be
`1` instead of `0`.
Fixes https://github.com/xamarin/xamarin-macios/issues/20503.
Calling the direct functions instead of sending messages is slightly faster,
and additionally it may make some static analyzers think we've enabled ARC for
our Objective-C code (which we don't, because we need to manually manage
reference counting).
These direct functions aren't in any public header (they're in a private header),
but they're documented as part of ARC here: https://clang.llvm.org/docs/AutomaticReferenceCounting.html#runtime-support,
and clang emits references to these methods from user code, so it should be safe for us to use them.
Fixes https://github.com/xamarin/xamarin-macios/issues/19413.
This also meant reviewing calling code to make sure that MonoObject*s are
released when they should be, which meant reviewing every method that returns
a MonoObject*, and release the result.
* Convert the GCHandles interface from 32-bit ints to pointer size types
This involves:
* Stop using some bits of the GCHandle to store extra flags, instead add an extra
field to store those flags.
* Define a INVALID_GCHANDLE constant and use it instead of 0/NULL. This is not
strictly required, but it makes the code more self-documenting.
* Define a GCHandle type (typedef'ed to void*) and change all variables and parameters
to use it instead of guint32.
* Use our own xamarin_gchandle_* methods (with pointer-sized types) that wraps
the mono_gchandle_* embedding API (which uses 32-bit types) everywhere.
* Update managed code (registrars, runtime code, etc) accordingly.
* [runtime] Make debug code compile.
* Fix typo.
* Fix signature of xamarin_create_gchandle.
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
Fixes these warnings:
shared.m:252:6: warning: 'OSAtomicDecrement32Barrier' is deprecated: first deprecated in watchOS 3.0 - Use std::atomic_fetch_sub() from <atomic> instead [-Wdeprecated-declarations]
if (OSAtomicDecrement32Barrier (&bl->descriptor->ref_count) == 0) {
^
/Applications/Xcode11.app/Contents/Developer/Platforms/WatchOS.platform/Developer/SDKs/WatchOS6.0.sdk/usr/include/libkern/OSAtomicDeprecated.h:201:9: note: 'OSAtomicDecrement32Barrier' has been explicitly marked deprecated here
int32_t OSAtomicDecrement32Barrier( volatile int32_t *__theValue );
^
shared.m:270:2: warning: 'OSAtomicIncrement32' is deprecated: first deprecated in watchOS 3.0 - Use std::atomic_fetch_add_explicit(std::memory_order_relaxed) from <atomic> instead [-Wdeprecated-declarations]
OSAtomicIncrement32 (&source->descriptor->ref_count);
^
/Applications/Xcode11.app/Contents/Developer/Platforms/WatchOS.platform/Developer/SDKs/WatchOS6.0.sdk/usr/include/libkern/OSAtomicDeprecated.h:171:9: note: 'OSAtomicIncrement32' has been explicitly marked deprecated here
int32_t OSAtomicIncrement32( volatile int32_t *__theValue );
^
monotouch-debug.m:309:10: warning: 'OSAtomicIncrement32Barrier' is deprecated: first deprecated in watchOS 3.0 - Use std::atomic_fetch_add() from <atomic> instead [-Wdeprecated-declarations]
int c = OSAtomicIncrement32Barrier (&http_send_counter);
^
/Applications/Xcode11.app/Contents/Developer/Platforms/WatchOS.platform/Developer/SDKs/WatchOS6.0.sdk/usr/include/libkern/OSAtomicDeprecated.h:182:9: note: 'OSAtomicIncrement32Barrier' has been explicitly marked deprecated here
int32_t OSAtomicIncrement32Barrier( volatile int32_t *__theValue );
^
* [runtime] Clean up public symbols. Fixes#5124.
Clean up public symbols, by:
* Symbols that don't need to be public (most of them), can be private.
* Prefix all public symbols with `xamarin_`.
* Add a test to ensure we don't introduce new public symbols.
* Use C symbols instead of mangled C++ symbols, since those are easier to
handle in the test.
This minimizes the chance of getting into a symbol clash with another native library.
Fixes https://github.com/xamarin/xamarin-macios/issues/5124.
* Some test fixes.
This warning:
shared.m:216:3: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
[pool release];
^~~~~~~~~~~~~~
is somewhat incorrect, because we're using NSAutoreleasePools in uncommon ways
(at the same time it's not entirely incorrect either, because we're not
following Apple's documentation about how to use NSAutoreleasePools).
Luckily calling [NSAutoreleasePool drain] is equivalent to [NSAutoreleasePool
release], so just replace the latter with the former to silence the warning,
since clang doesn't know those two are equivalent.
In bug #43592 the following occurs:
* App calls an API that takes a block.
* We create a stack-based ObjC block based on the delegate the app provided.
This block has a pointer to a block description, describing the block
in question (including the signature of the block, as an ObjC-type
encoded string). We allocate a new block description for every block.
* Apple's API stores the pointer to the signature string somewhere.
* Apple calls _Block_copy to get a heap-based block.
* We create a heap-based block, and copy the entire description into the
new heap-based block (including a copy of the signature).
* Apple returns from the API, and then we free the stack-based block
(and the descriptor, and thus the signature string in the descriptor).
* Apple uses the pointer to the signature stored previously to investigate
the signature of the block, and crashes because this signature has been
freed.
The assumption in Apple's code is that the description will never be freed,
which is true for any Xcode project (clang will always be able to create the
block description at compile-time and emit it in the binary, which means the
memory will never be freed). We could potentially do the same thing in the
static registrar, but we'd still need a solution when using the dynamic
registrar.
To fix this instead of copying the entire description structure when creating
a heap-based block from the stack-based block, we make the description ref-
counted, and just use the same description in the heap-based block.
The signature will now stay in memory until both the heap-based and stack-
based blocks have been freed, and we hope Apple doesn't have any API that
needs the signature after all the blocks for that signature have been freed.
https://bugzilla.xamarin.com/show_bug.cgi?id=43592
Rolf Bjarne Kvinge
Manuel de la Pena
Zoltan Varga