Skip to content

Commit 3a9e4a9

Browse files
authored
[runtime] Fix storing additional data for NSObject in native code. Fixes #23684. (#23780)
* Implement the 'xamarinGetNSObjectData' method for both dynamic and static registrar, so that it's actually possible to get the additional data. * Don't overwrite a pointer to valid data with a NULL pointer. * Add a test. Fixes #23684.
1 parent ecbf05a commit 3a9e4a9

File tree

13 files changed

+216
-5
lines changed

13 files changed

+216
-5
lines changed

runtime/runtime.m

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@
122122
void* get_flags_tramp;
123123
void* set_flags_tramp;
124124
void* retainWeakReference_tramp;
125+
void* get_nsobject_data_tramp;
125126
};
126127

127128
enum InitializationFlags : int {
@@ -182,6 +183,7 @@
182183
(void *) &xamarin_get_flags_trampoline,
183184
(void *) &xamarin_set_flags_trampoline,
184185
(void *) &xamarin_retainWeakReference_trampoline,
186+
(void *) &xamarin_get_nsobject_data_trampoline,
185187
};
186188

187189
static struct InitializationOptions options = { 0 };
@@ -916,7 +918,10 @@ -(struct NSObjectData*) xamarinGetNSObjectData;
916918
{
917919
/* This is called with the GC lock held, so it can only use signal-safe code */
918920
struct Managed_NSObject *obj = (struct Managed_NSObject *) object;
919-
//PRINT ("In finalization response for %s.%s %p (handle: %p class_handle: %p flags: %i)\n",
921+
#if defined(DEBUG_REF_COUNTING)
922+
// Don't use PRINT/NSLog/os_log here, because the process can end up aborting due to recursive locks inside the ObjC runtime. Luckily fprintf works fine.
923+
fprintf (stderr, "object_queued_for_finalization (%p): data=%p flags=%x handle=%p\n", object, obj->data, (unsigned int) (obj->data ? obj->data->flags : 0), obj->data ? obj->data->handle : NULL);
924+
#endif
920925
if (obj->data)
921926
obj->data->flags |= NSObjectFlagsInFinalizerQueue;
922927
}

runtime/trampolines.m

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -837,8 +837,10 @@
837837
bool isInFinalizerQueue = (flags & NSObjectFlagsInFinalizerQueue) == NSObjectFlagsInFinalizerQueue;
838838

839839
#if defined(DEBUG_REF_COUNTING)
840-
PRINT ("xamarin_retainWeakReference_trampoline (%s Handle=%p) flags: %x isInFinalizerQueue: %i\n",
841-
class_getName ([self class]), self, flags, isInFinalizerQueue);
840+
struct NSObjectData *data = xamarin_get_nsobject_data (self);
841+
// Don't use PRINT/NSLog/os_log here, because the process can end up aborting due to recursive locks inside the ObjC runtime. Luckily fprintf works fine.
842+
fprintf (stderr, "xamarin_retainWeakReference_trampoline (%s Handle=%p) flags: %x isInFinalizerQueue: %i data: %p data->flags: %x data->handle: %p\n",
843+
class_getName ([self class]), self, flags, isInFinalizerQueue, data, (unsigned int) (data ? data->flags : 0), data ? data->handle : NULL);
842844
#endif
843845

844846
// Do not allow any weak references to be resolved if the managed wrapper has been scheduled for finalization,
@@ -896,7 +898,8 @@
896898

897899
if (obj != NULL) {
898900
obj->gc_handle = gc_handle;
899-
obj->data = NULL;
901+
if (data != NULL)
902+
obj->data = data;
900903
obj->gchandle_flags = gchandle_flags;
901904
}
902905

@@ -965,6 +968,24 @@
965968
pthread_mutex_unlock (&gchandle_hash_lock);
966969
}
967970

971+
struct NSObjectData *
972+
xamarin_get_nsobject_data_trampoline (id self, SEL sel)
973+
{
974+
/* This is for types registered using the dynamic registrar */
975+
XamarinAssociatedObject *obj;
976+
obj = objc_getAssociatedObject (self, associated_key);
977+
978+
struct NSObjectData *data = NULL;
979+
if (obj != NULL)
980+
data = obj->data;
981+
982+
#if defined(DEBUG_REF_COUNTING)
983+
PRINT ("xamarin_get_nsobject_data_trampoline (%s = %p) obj: %p data: %p data->flags: %x data->handle: %p\n",
984+
class_getName ([self class]), self, obj, data ? data->flags : NULL, data ? data->handle : NULL);
985+
#endif
986+
987+
return data;
988+
}
968989
id
969990
xamarin_generate_conversion_to_native (MonoObject *value, MonoType *inputType, MonoType *outputType, MonoMethod *method, void *context, GCHandle *exception_gchandle)
970991
{

runtime/xamarin/runtime.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,8 @@ struct NSObjectData {
370370
id handle;
371371
struct objc_super* super;
372372
uint32_t /* NSObjectFlags */ flags;
373+
// if this structure ever changes, the encoding for this method will likely have to be updated in Registrar.RegistrarTypeUnsafe, currently it's:
374+
// Signature = "^{NSObjectData=@^{objc_super}I}:",
373375
};
374376

375377
#ifdef __cplusplus

runtime/xamarin/trampolines.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ GCHandle xamarin_get_gchandle_trampoline (id self, SEL sel);
4343
bool xamarin_set_gchandle_trampoline (id self, SEL sel, GCHandle gc_handle, enum XamarinGCHandleFlags flags, struct NSObjectData *data);
4444
enum XamarinGCHandleFlags xamarin_get_flags_trampoline (id self, SEL sel);
4545
void xamarin_set_flags_trampoline (id self, SEL sel, enum XamarinGCHandleFlags flags);
46+
struct NSObjectData * xamarin_get_nsobject_data_trampoline (id self, SEL sel);
4647

4748
int xamarin_get_frame_length (id self, SEL sel);
4849
bool xamarin_collapse_struct_name (const char *type, char struct_name[], int max_char, GCHandle *exception_gchandle);

src/ObjCRuntime/DynamicRegistrar.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,9 @@ bool RegisterMethod (ObjCMethod method)
11341134
case Trampoline.RetainWeakReference:
11351135
tramp = Method.RetainWeakReferenceTrampoline;
11361136
break;
1137+
case Trampoline.GetNSObjectData:
1138+
tramp = Method.GetNSObjectDataTrampoline;
1139+
break;
11371140
default:
11381141
throw ErrorHelper.CreateError (4144, "Cannot register the method '{0}.{1}' since it does not have an associated trampoline. Please file a bug report at https://github.com/dotnet/macios/issues/new", method.DeclaringType.Type.FullName, method.Name);
11391142
}

src/ObjCRuntime/Method.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ internal unsafe static IntPtr SetGCHandleFlagsTrampoline {
112112
internal unsafe static IntPtr RetainWeakReferenceTrampoline {
113113
get { return Runtime.options->Trampolines->retainWeakReference_tramp; }
114114
}
115+
116+
internal unsafe static IntPtr GetNSObjectDataTrampoline {
117+
get { return Runtime.options->Trampolines->get_nsobject_data_tramp; }
118+
}
115119
#endif // !COREBUILD
116120
}
117121
}

src/ObjCRuntime/Registrar.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ public override bool IsImplicit {
615615
case Trampoline.GetGCHandleFlags:
616616
case Trampoline.SetGCHandleFlags:
617617
case Trampoline.RetainWeakReference:
618+
case Trampoline.GetNSObjectData:
618619
return true;
619620
default:
620621
return false;
@@ -2119,6 +2120,13 @@ ObjCType RegisterTypeUnsafe (TType type, ref List<Exception> exceptions)
21192120
Signature = $"{GetBoolEncoding ()}@:",
21202121
IsStatic = false,
21212122
}, ref exceptions);
2123+
2124+
objcType.Add (new ObjCMethod (this, objcType, null) {
2125+
Selector = "xamarinGetNSObjectData",
2126+
Trampoline = Trampoline.GetNSObjectData,
2127+
Signature = "^{NSObjectData=@^{objc_super}I}:",
2128+
IsStatic = false,
2129+
}, ref exceptions);
21222130
}
21232131

21242132
// Find conform_to_protocol
@@ -2854,5 +2862,6 @@ enum Trampoline {
28542862
GetGCHandleFlags,
28552863
SetGCHandleFlags,
28562864
RetainWeakReference,
2865+
GetNSObjectData,
28572866
}
28582867
}

src/ObjCRuntime/Runtime.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ internal struct Trampolines {
165165
public IntPtr get_gchandle_flags_tramp;
166166
public IntPtr set_gchandle_flags_tramp;
167167
public IntPtr retainWeakReference_tramp;
168+
public IntPtr get_nsobject_data_tramp;
168169
}
169170
#pragma warning restore 649
170171

tests/bindings-test/ApiDefinition.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,4 +573,19 @@ interface VeryGenericFactory {
573573
[Static]
574574
IVeryGenericConsumerProtocol GetConsumer ();
575575
}
576+
577+
[BaseType (typeof (NSObject))]
578+
interface WeakReferencedObject {
579+
[Export ("doSomething")]
580+
int DoSomething ();
581+
}
582+
583+
[BaseType (typeof (NSObject))]
584+
interface WeakReferenceHolder {
585+
[Export ("addObject:")]
586+
void AddObject (WeakReferencedObject obj);
587+
588+
[Export ("callDoSomething:nonNilObjectCount:gotExpectedResponse:gotUnexpectedResponse:gotFinalizedResponse:")]
589+
void CallDoSomething (ref int nilObjectCount, ref int nonNilObjectCount, ref int gotExpectedResponse, ref int gotUnexpectedResponse, ref int gotFinalizedResponse);
590+
}
576591
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
using System;
2+
using System.Diagnostics;
3+
using System.Threading;
4+
5+
using Foundation;
6+
using ObjCRuntime;
7+
8+
using Bindings.Test;
9+
10+
using NUnit.Framework;
11+
12+
using Xamarin.Utils;
13+
14+
namespace MonoTouchFixtures.ObjCRuntime {
15+
[TestFixture]
16+
[Preserve (AllMembers = true)]
17+
public class NativeWeakTest {
18+
[Test]
19+
public void TestWeakReferences ()
20+
{
21+
var start = Stopwatch.StartNew ();
22+
23+
var gcThread = new Thread (() => {
24+
while (start.Elapsed.TotalSeconds < 5) {
25+
GC.Collect ();
26+
Thread.Sleep (100);
27+
}
28+
}) {
29+
IsBackground = true,
30+
};
31+
gcThread.Start ();
32+
33+
int nilObjectCount = 0;
34+
int nonNilObjectCount = 0;
35+
int gotExpectedResponse = 0;
36+
int gotUnexpectedResponse = 0;
37+
int gotFinalizedResponse = 0;
38+
39+
const int objectCount = 100;
40+
41+
var creatorThread = new Thread (() => {
42+
using var holder = new WeakReferenceHolder ();
43+
for (var i = 0; i < objectCount; i++) {
44+
holder.AddObject (new MyWeakReferencedObject ());
45+
}
46+
GC.Collect ();
47+
GC.WaitForPendingFinalizers ();
48+
49+
holder.CallDoSomething (ref nilObjectCount, ref nonNilObjectCount, ref gotExpectedResponse, ref gotUnexpectedResponse, ref gotFinalizedResponse);
50+
}) {
51+
IsBackground = true,
52+
};
53+
creatorThread.Start ();
54+
55+
Assert.That (creatorThread.Join (TimeSpan.FromSeconds (15)), "Join CreatorThread");
56+
57+
var msg = $"Nil object count: {nilObjectCount} Non-nil object count: {nonNilObjectCount} Expected response: {gotExpectedResponse} Unexpected responses: {gotUnexpectedResponse} Finalized response: {gotFinalizedResponse}";
58+
Assert.Multiple (() => {
59+
Assert.That (nonNilObjectCount, Is.EqualTo (objectCount - nilObjectCount), $"Non-nil object count: {msg}");
60+
Assert.That (gotExpectedResponse, Is.EqualTo (objectCount - nilObjectCount), $"Expected response count: {msg}");
61+
Assert.That (gotUnexpectedResponse, Is.EqualTo (0), $"Unexpected response count: {msg}");
62+
Assert.That (gotFinalizedResponse, Is.EqualTo (0), $"Responses after finalization: {msg}");
63+
});
64+
}
65+
}
66+
67+
class MyWeakReferencedObject : WeakReferencedObject {
68+
bool finalized;
69+
70+
public override int DoSomething ()
71+
{
72+
return finalized ? 314 : 42;
73+
}
74+
75+
~MyWeakReferencedObject ()
76+
{
77+
finalized = true;
78+
}
79+
}
80+
}

0 commit comments

Comments
 (0)