Nicholas Clark <n...@ccl4.org> wrote .. > In MVM_hll_set_config(), the call to MVM_string_utf8_decode() inside > check_config_key() causes allocation, which means that it can cause GC > runs. Which can mean that the object pointed to by config_hash can move. > Indeed.
> 1) Is the attached patch the correct way to do this? Yes, and I've applied it. > 2) How many more of these lurk - is there a systematic way to find these? Maybe with some kind of static analysis (mark calls that may/may not allocate, and make sure any variable referenced after such a call was MVMROOT'd), but my suspicion is we'll get a lot of false positives without lots of tuning. On the other hand, if it's a review "todo list" then that may not matter so much. > 3) Is it the case that any MoarVM API call *might* allocate (even if the > implementation doesn't yet), hence correct code has to assume that > i) there might be a GC run triggered within any and every API call > ii) any pointer to a MoarVM collectable might be in the nursery and hence > might move If (i) is true then (ii) should always be assumed. At the moment gen2 does not ever move things, but we may not always keep that promise into the future. While it's OK to rely on the current state *inside* of the VM (and we do), it'd be best to strongly discourage relying on it from an embedding perspective. > unless, and only unless, the code can prove that it doesn't need to? > > Should MoarVM API calls that can't trigger GC be explicitly documented as > such? > I was more inclined to mark those that *can* trigger it, but maybe a "safe list" is better. Either way, I think they should end up as annotations in the source code (MVM_NO_GC_ALLOC or so on the function declaration) rather than as separate documentation. Thanks, Jonathan
