Don't use mono_gc_is_moving for incremental gc

[jechter]

When I added incremental bdwgc support to mono, I used the existing mono_gc_is_moving function from the mono sgen implementation to make sure that mono would generate write barriers everywhere it is needed. But returning true from mono_gc_is_moving does more than just enabling write barriers. It would also in several situations use indirect handles for GC objects, as those might be moved. This is not needed for incremental bdwgc, so it is an unnecessary cost. But, more importantly, it turned out that some memory referred to by such handles would not be freed after domain reloads, causing massive memory leaks in the unity editor.

I believe the specific case where such objects would leak is when generating code to refer to static readonly objects. In the non-moving case, mono would just write the pointer value into the emitted code (since it would never change), in the moving case, it would create an indirect handle. It appeared that this handle would not be released, and such objects would thus leak. I don't know if this is a bug in sgen as well (possible, because I guess not many users of mono other then Unity rely on domain reloads) or if that is specific to using this code in bdwgc.

But anyways, the fact is that this is not needed for incremental bdwgc. So, now we no longer return true from mono_gc_is_moving in the incremental bdwgc case, and instead also check for mono_gc_is_incremental in any case where this check is used to implement write barriers. This fixes the memory leaks in the Unity editor.

[jechter]

This will need backports to 19.3 and 19.2

[joncham]

Looks good. Longer term it would be nice to have a specific function for needed check, like mono_gc_needs_write_barriers().

[jechter]

Looks good. Longer term it would be nice to have a specific function for needed check, like mono_gc_needs_write_barriers().

That's a good point, that will make it safer in the future - will change it.