Skip to content

[Bgen] Ensure that GC.KeepAlive is called in extension methods. #22433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 23, 2025
Merged

[Bgen] Ensure that GC.KeepAlive is called in extension methods. #22433

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
360f46b
[Bgen] Ensure that GC.KeepAlive is called in extension methods.
mandel-macaque Mar 20, 2025
c5afe1e
Apply suggestions from code review
mandel-macaque Mar 20, 2025
c1426d5
Address code reviews.
mandel-macaque Mar 20, 2025
ca0e9b0
Auto-format source code
Mar 20, 2025
cddc790
Apply suggestions from code review
mandel-macaque Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 40 additions & 15 deletions src/bgen/Generator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2899,9 +2899,11 @@ void GetReturnsWrappers (MethodInfo mi, MemberInformation minfo, Type declaringT
}
}

void GenerateInvoke (bool stret, bool supercall, MethodInfo mi, MemberInformation minfo, string selector, string args, bool assign_to_temp, Type category_type, bool aligned)
void GenerateInvoke (bool stret, bool supercall, MethodInfo mi, MemberInformation minfo, string selector, string args, Type category_type, bool aligned)
{
string target_name = (category_type is null && !minfo.is_extension_method && !minfo.is_protocol_implementation_method) ? "this" : "This";
var isInstanceMethod = category_type is null && !minfo.is_extension_method &&
!minfo.is_protocol_implementation_method;
string target_name = isInstanceMethod ? "this" : "This";
string handle = supercall ? ".SuperHandle" : ".Handle";

// If we have supercall == false, we can be a Bind method that has a [Target]
Expand Down Expand Up @@ -2958,7 +2960,7 @@ void GenerateInvoke (bool stret, bool supercall, MethodInfo mi, MemberInformatio
print ($"IntPtr {handleName};");
print ($"{handleName} = global::{NamespaceCache.Messaging}.IntPtr_objc_msgSend (Class.GetHandle (typeof (T)), Selector.GetHandle (\"alloc\"));");
print ($"{handleName} = {sig} ({handleName}, {selector_field}{args});");
print ($"{(assign_to_temp ? "ret = " : "return ")} global::ObjCRuntime.Runtime.GetINativeObject<T> ({handleName}, true);");
print ($"ret = global::ObjCRuntime.Runtime.GetINativeObject<T> ({handleName}, true);");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we don't need the assign_to_temp variable/parameter, so it can be removed.

} else {
bool returns = mi.ReturnType != TypeCache.System_Void && mi.Name != "Constructor";
string cast_a = "", cast_b = "";
Expand All @@ -2973,20 +2975,26 @@ void GenerateInvoke (bool stret, bool supercall, MethodInfo mi, MemberInformatio

if (minfo.is_static)
print ("{0}{1}{2} (class_ptr, {5}{6}){7};",
returns ? (assign_to_temp ? "ret = " : "return ") : "",
returns ? "ret = " : "",
cast_a, sig, target_name,
"/*unusued3*/", //supercall ? "Super" : "",
selector_field, args, cast_b);
else
print ("{0}{1}{2} ({3}{4}, {5}{6}){7};",
returns ? (assign_to_temp ? "ret = " : "return ") : "",
returns ? "ret = " : "",
cast_a, sig, target_name,
handle,
selector_field, args, cast_b);

if (postproc.Length > 0)
print (postproc.ToString ());
}

if (!isInstanceMethod) {
// if this is a extension of any kind, ensure that we keep alive the this parameter
// so that it is not collected before the msg send call has completed.
print ("GC.KeepAlive (This);");
}
}

void GenerateNewStyleInvoke (bool supercall, MethodInfo mi, MemberInformation minfo, string selector, string args, bool assign_to_temp, Type category_type)
Expand All @@ -2999,15 +3007,15 @@ void GenerateNewStyleInvoke (bool supercall, MethodInfo mi, MemberInformation mi
if (x64_stret) {
print ("if (global::ObjCRuntime.Runtime.IsARM64CallingConvention) {");
indent++;
GenerateInvoke (false, supercall, mi, minfo, selector, args, assign_to_temp, category_type, false);
GenerateInvoke (false, supercall, mi, minfo, selector, args, category_type, false);
indent--;
print ("} else {");
indent++;
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && x64_stret);
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, category_type, aligned && x64_stret);
indent--;
print ("}");
} else {
GenerateInvoke (false, supercall, mi, minfo, selector, args, assign_to_temp, category_type, false);
GenerateInvoke (false, supercall, mi, minfo, selector, args, category_type, false);
}
return;
}
Expand All @@ -3022,37 +3030,37 @@ void GenerateNewStyleInvoke (bool supercall, MethodInfo mi, MemberInformation mi
// First check for arm64
print ("if (global::ObjCRuntime.Runtime.IsARM64CallingConvention) {");
indent++;
GenerateInvoke (false, supercall, mi, minfo, selector, args, assign_to_temp, category_type, false);
GenerateInvoke (false, supercall, mi, minfo, selector, args, category_type, false);
indent--;
// If we're not arm64, but we're 64-bit, then we're x86_64
print ("} else if (IntPtr.Size == 8) {");
indent++;
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && x64_stret);
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, category_type, aligned && x64_stret);
indent--;
// if we're not 64-bit, but we're on device, then we're 32-bit arm
print ("} else if (Runtime.Arch == Arch.DEVICE) {");
indent++;
GenerateInvoke (arm_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && arm_stret);
GenerateInvoke (arm_stret, supercall, mi, minfo, selector, args, category_type, aligned && arm_stret);
indent--;
// if we're none of the above, we're x86
print ("} else {");
indent++;
GenerateInvoke (x86_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && x86_stret);
GenerateInvoke (x86_stret, supercall, mi, minfo, selector, args, category_type, aligned && x86_stret);
indent--;
print ("}");
} else {
print ("if (IntPtr.Size == 8) {");
indent++;
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && x64_stret);
GenerateInvoke (x64_stret, supercall, mi, minfo, selector, args, category_type, aligned && x64_stret);
indent--;
print ("} else {");
indent++;
GenerateInvoke (x86_stret, supercall, mi, minfo, selector, args, assign_to_temp, category_type, aligned && x86_stret);
GenerateInvoke (x86_stret, supercall, mi, minfo, selector, args, category_type, aligned && x86_stret);
indent--;
print ("}");
}
} else {
GenerateInvoke (false, supercall, mi, minfo, selector, args, assign_to_temp, category_type, false);
GenerateInvoke (false, supercall, mi, minfo, selector, args, category_type, false);
}
}

Expand Down Expand Up @@ -3554,6 +3562,17 @@ public void GenerateMethodBody (MemberInformation minfo, MethodInfo mi, string s
var nullableReturn = isClassType ? "?" : string.Empty;
print ("{0}{1} ret;", TypeManager.FormatType (minfo.type, mi.ReturnType), nullableReturn);
}
} else if (mi.ReturnType != TypeCache.System_Void && mi.Name != "Constructor") {
if (minfo.is_bindAs) {
var bindAsAttrib = GetBindAsAttribute (minfo.mi);
// tricky, e.g. when an nullable `NSNumber[]` is bound as a `float[]`, since FormatType and bindAsAttrib have not clue about the original nullability
print ("{0} ret;", TypeManager.FormatType (bindAsAttrib.Type.DeclaringType, bindAsAttrib.Type));
} else {
print ("{0} ret;", TypeManager.FormatType (minfo.type, mi.ReturnType));
}
} else if (minfo.is_ctor && minfo.is_protocol_member) {
// special case because constructors in protocol members will be converted to factory methods
print ($"T? ret;");
}

bool needs_temp = use_temp_return || disposes.Length > 0;
Expand Down Expand Up @@ -3666,6 +3685,12 @@ public void GenerateMethodBody (MemberInformation minfo, MethodInfo mi, string s
// we can't be 100% confident that the ObjC API annotations are correct so we always null check inside generated code
print ("return ret!;");
}
} else if (minfo.is_ctor && minfo.is_protocol_member) {
// special case since ctrs in protocol members become create methods
print ("return ret;");
} else if (mi.ReturnType != TypeCache.System_Void && mi.Name != "Constructor") {
// general case if we do return and we are not a constructor.
print ("return ret;");
}
if (minfo.is_ctor)
WriteMarkDirtyIfDerived (sw, mi.DeclaringType);
Expand Down
Loading