Pass ObjectReference to the is_movable API

[k-sareen]

This allows policies that implement object pinning (such as ImmixSpace) to return a more accurate answer.

[wks]

While I agree that the trait method SFT::is_movable should have object as a one parameter (because a space can, in theory, refuse to move some objects but not others), I don't think object pinning should be involved here.

The main use of ObjectReference::is_movable is foreign function interface (FFI) where the VM passes an object reference (or interior pointer) to native code. If is_movable returns true, the VM will omit the pinning and unpinning operation around the native call (or the action of telling mmtk-core the object is an unmovable root when GC happens while the native function is being executed).

But if is_movable answers true because the pinning bit is set, the following thing may happen. Assume there are two mutator threads.

(Update: I got the return values wrong. Now corrected.)

1. The first mutator is about to call a native function. It calls obj.is_movable and gets false true (it is movable). Then the first mutator pins the object and then calls the native function.
2. The second mutator is about to call another function. It calls obj.is_movable and gets true false (it is not movable) because the first mutator pinned that object. It omits the pinning operation and calls the native function directly.
3. The first mutator returns from the native function, and unpins the object.
4. GC started. The GC thinks all mutators have stopped because the second mutator is executing native code. The GC thread sees the object is not pinned, and moves it.
5. The second mutator is still accessing the object in native code, but the object is moved. It is a data race.

The documentation of ObjectReference::is_movable simply says "Can the object be moved?" It is not very clear about how long the answer remain valid. Intuitively, if ObjectReference::is_movable returns true, it should mean "the object will never ever move until it is dead". But if the object cannot move because the pinning bit is set, it can once again become movable after we remove the pinning bit. This may cause some subtle confusion. We need to fix the documentation so that the VM binding developers know what to expect.

[k-sareen]

I think you have the return values swapped in your example. If the object is pinned then is_movable returns false.

I agree that this is a valid use-case, but the documentation of is_movable in the SFT mentions object pinning. It is not the only use-case of is_movable, however. ART uses it when cloning objects, for example, to allocate the object with the correct semantics/allocator. I've just removed the non-moving allocator and replaced it with pin bits, and I tripped on this bug.

I'm not sure what the best way to approach this is.

[k-sareen]

Actually, your example is true for the current state of affairs anyway. As in, if two threads pin the same object, then one thread may unpin it while the other thread is still using it. So this PR doesn't make the situation worse. I'm not sure if that is something we can resolve anyway as we would effectively need to implement a counter for the pinning metadata instead of a single bit.

[qinsoon]

The scenario Kunshan describes is an issue with our pinning semantic (which uses a boolean rather than a count). If you replace the call to object.is_movable with memory_manager::is_pinned in the scenario, you would get the same problem.

[k-sareen]

@wks What are your thoughts?

[wks]

I think the pinning counter and the semantics of is_movable are orthogonal.

If we implement a per-object pinning counter and apply pin_object and unpin_object unconditionally when calling native, there will be no problem.

But if we use pinning counter, and also use is_movable to detect if an object is movable and elide the pin/unpin operation depending on the return value of is_movable, there will still be a problem. The scenario I described will still go wrong if the first mutator applies a pinning counter, and the second mutator elides the pinning/unpinning because it is "unmovable".

The crux is still the semantics. I insist that object.is_movable returning false should mean it can't be moved until it dies.

But if you need to clone object and also clone some special semantics (e.g. whether it can be moved), we have to find other metadata to use.

Ruby also has a dup function that clones an object. It also copies the "write-barrier-unprotected" (WB-unprotected) property and its finalizers. CRuby uses a bitmap to identify if an object is "WB-unprotected", and uses an in-header bit for whether it is finalizable. Currently I just implemented the query of whether an object is "WB-unprotected" using a table lookup, which is as efficient as CRuby.

[wks]

BTW, if you are sure there is no race, you can use memory_manager::is_pinned to query the pinning bit and then pin the copied object, too, if that's desired.

[k-sareen]

Okay. I don't think this will get resolved over GitHub comments. It doesn't matter in my case since I never unpin objects (until they die), so I'll just keep this in my branch.