Maybe should compare with GetMaxGeneration.
It will be 2 for the foreseeable future, but it might be cleaner to not assume that,

Hmmmm. I may want to chat offline about this. I think I need to get more educated in this area first.

From what I see in the code, you check for the max generation just to be sure it is not nested. That seems correct and practically it will be 2.
On the other hand GetMaxGeneration would be cheap enough to call once per-GC and not have a literal in the code.

perhaps even simpler would be to move Interop::OnGCStarted invocation under if (condemned == max_gen) in the GcStartWork

Note that xamarin-macios will take a global lock inside this callback: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/runtime.m#L337 .

Taking this global lock while the background GC is in progress will frequently turn the background GC into blocking GC. The execution will tend to get blocked on this global lock that will be only released once the background GC is finished.

This may be as a shortcut to just hack some prototype together, but I do not think it acceptable as a shipping design.

I am not very familiar with the purpose - is this to block prevent something while GC in progress or to do something while GC is in progress? I think it might be the second, since we ignore ephemeral GCs.

Then, perhaps this begin/end should be around the blocking stage of background GC (when we do dependent handles and such) , or around entire GC when it is a blocking gen2.

This is one of the reasons: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/runtime.m#L1941-L2006

@rolfbjarne Yep. That one i have been able to understand - the comments are very helpful. The one I am more confused about is https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L112-L114. This one, https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L103-L106, seems similar but doesn't have the lock so the use case isn't obvious to me.

If anybody can think of another solution than the lock, I'm happy to try it out.

but that's not a problem after retain

@rolfbjarne Okay. So I get everything you said above. But if that is true, then what is the purpose of surrounding the call with the lock? It would seem that locking isn't going to ensure that i isn't deallocated here.

xamarin_framework_peer_lock ();
[i retain];
xamarin_framework_peer_unlock ();

I started by looking for all instances where the "peer lock" was utilized. There seem to be two instances. The one above and the one in the managed ref release function. The one above is confusing me because it is a retain and not a release.

Yeah, that's a retain with a problem too, that goes like this:

a1) Thread A starts a GC
a2) Thread A reads the retainCount of object i in the toggle ref callback
a3) Thread A collects everything collectible.
b1) Thread B retains i
b2) Thread B returns from the function xamarin_marshal_return_value_impl (leaving no other references to the managed object on the stack).

The race condition goes like this:

a1. GC starts
a2. The toggle ref callback sees that retainCount = 1, so according to the toggle ref callback the object is collectible)
b1. retainCount is now 2.
b2. The managed i disappears from the stack.
a3. The GC now collects i (because it's nowhere to be seen on the stack), and it has a retainCount of 2.

This isn't a problem here: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L103-L106, because in that case there is no managed wrapper object for the NSArray in question, we're just marshaling a managed array to an NSArray directly.

Spoke offline. The above scenario is described as follows:

Conceptually, it's these actions:

1. Create managed wrapper object for a native object.
2. Create a handle to this managed object in native code in some form (for Mono we use a MonoObject* pointer, but it can be any kind of representation), where this handle is the only thing keeping the GC from collecting the managed object.
3. Call retain on the native object.
4. Release/destroy the handle from 2.
5. The toggleref callback checks the retainCount of the native object.
6. The GC collects everything it can.

The following order is the race condition:

1-2-5-3-4-6

1. Create managed wrapper object for a native object.
2. Create a handle to this managed object in native code in some form (for Mono we use a MonoObject* pointer on the stack, but it can be any kind of representation), where this handle is the only thing keeping the GC from collecting the managed object.
3. The toggleref callback checks the retainCount of the native object (getting 1 and telling the GC that native code does not have a reference to the managed object).
4. Call retain on the native object.
5. Release/destroy the handle from 2.
6. The GC collects everything it can (which will collect the managed object, because the toggle ref callback said "don't need it", and there's nothing else keeping it alive)

Create a handle to this managed object in native code in some form (for Mono we use a MonoObject* pointer

Is it important that the handle is created in native code? In other words, is this any different from just keeping the managed object reference in managed code? There should be no difference for CoreCLR. There may be a difference for Mono due to how conservative GC stack scanning and embedding APIs work.

No, it doesn't matter exactly where the handle is created, but I think that's missing the point.

The problem is the synchronization between releasing that handle, and code in the toggle ref callback. For Mono the handle is implicitly released by returning from the stack frame (this line of code makes sure the handle is considered alive up until that point: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L118 - other types of handles might need explicit release at that point).

Let's say we use a GCHandle (where the managed function returned a GCHandle of the managed object that was created), in which case this code block:

https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L108-L119

could look something like this (retval_gchandle is the GCHandle the managed function returned):

id i = xamarin_get_handle (retval_gchandle, exception_gchandle);
if (*exception_gchandle != 0) {
	mono_gchandle_free (retval_gchandle);
	return NULL;
}

xamarin_framework_peer_lock ();
[i retain];
xamarin_framework_peer_unlock ();
if (!retain)
	[i autorelease];

mono_gchandle_free (retval_gchandle);
return i;

Without the lock, the following scenario is possible:

a) Thread A is just about to execute [i retain].
b) Thread B is running the GC, and calls the toggle ref callback.
c) Thread B: The toggle ref callback sees that the retainCount of i is 1, and tells the GC that the object can be collected.
d) Thread A calls[i retain], to prevent the object from being collected, and frees retval_gchandle.
e) Thread B: The GC collects the object.

not an immediate concern, it's a few steps ahead, but we'll need maccatalyst too to support MAUI (and likely Blazor) too
dotnet/designs#174

Yeah, that's a retain with a problem too, that goes like this:

a1) Thread A starts a GC
a2) Thread A reads the retainCount of object i in the toggle ref callback
a3) Thread A collects everything collectible.
b1) Thread B retains i
b2) Thread B returns from the function xamarin_marshal_return_value_impl (leaving no other references to the managed object on the stack).

The race condition goes like this:

a1. GC starts
a2. The toggle ref callback sees that retainCount = 1, so according to the toggle ref callback the object is collectible)
b1. retainCount is now 2.
b2. The managed i disappears from the stack.
a3. The GC now collects i (because it's nowhere to be seen on the stack), and it has a retainCount of 2.

This isn't a problem here: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L103-L106, because in that case there is no managed wrapper object for the NSArray in question, we're just marshaling a managed array to an NSArray directly.

No, it doesn't matter exactly where the handle is created, but I think that's missing the point.

The problem is the synchronization between releasing that handle, and code in the toggle ref callback. For Mono the handle is implicitly released by returning from the stack frame (this line of code makes sure the handle is considered alive up until that point: https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L118 - other types of handles might need explicit release at that point).

Let's say we use a GCHandle (where the managed function returned a GCHandle of the managed object that was created), in which case this code block:

https://github.com/xamarin/xamarin-macios/blob/daa7e123e212eb3c9e0f30d951b66c0fe254d754/runtime/trampolines.m#L108-L119

could look something like this (retval_gchandle is the GCHandle the managed function returned):

id i = xamarin_get_handle (retval_gchandle, exception_gchandle);
if (*exception_gchandle != 0) {
	mono_gchandle_free (retval_gchandle);
	return NULL;
}

xamarin_framework_peer_lock ();
[i retain];
xamarin_framework_peer_unlock ();
if (!retain)
	[i autorelease];

mono_gchandle_free (retval_gchandle);
return i;

Without the lock, the following scenario is possible:

a) Thread A is just about to execute [i retain].
b) Thread B is running the GC, and calls the toggle ref callback.
c) Thread B: The toggle ref callback sees that the retainCount of i is 1, and tells the GC that the object can be collected.
d) Thread A calls[i retain], to prevent the object from being collected, and frees retval_gchandle.
e) Thread B: The GC collects the object.