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.
1) Is the attached patch the correct way to do this?
2) How many more of these lurk - is there a systematic way to find these?
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
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?
Nicholas Clark
>From 1b046a7a2832171bd2e50399410396365a6634dd Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Wed, 25 Dec 2013 21:57:20 +0100
Subject: [PATCH 1/2] In MVM_hll_set_config() need to root config_hash, as GC might move it.
The calls to MVM_string_utf8_decode() allocate, which means that they can
trigger GC.
---
src/core/hll.c | 31 +++++++++++++++++--------------
1 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/core/hll.c b/src/core/hll.c
index 83c2673..2a2d275 100644
--- a/src/core/hll.c
+++ b/src/core/hll.c
@@ -73,20 +73,23 @@ MVMObject * MVM_hll_set_config(MVMThreadContext *tc, MVMString *name, MVMObject
MVM_exception_throw_adhoc(tc, "set hll config needs concrete hash");
}
- check_config_key(tc, config_hash, "int_box", int_box_type, config);
- check_config_key(tc, config_hash, "num_box", num_box_type, config);
- check_config_key(tc, config_hash, "str_box", str_box_type, config);
- check_config_key(tc, config_hash, "slurpy_array", slurpy_array_type, config);
- check_config_key(tc, config_hash, "slurpy_hash", slurpy_hash_type, config);
- check_config_key(tc, config_hash, "array_iter", array_iterator_type, config);
- check_config_key(tc, config_hash, "hash_iter", hash_iterator_type, config);
- check_config_key(tc, config_hash, "foreign_type_int", foreign_type_int, config);
- check_config_key(tc, config_hash, "foreign_type_num", foreign_type_num, config);
- check_config_key(tc, config_hash, "foreign_type_str", foreign_type_str, config);
- check_config_key(tc, config_hash, "foreign_transform_array", foreign_transform_array, config);
- check_config_key(tc, config_hash, "foreign_transform_hash", foreign_transform_hash, config);
- check_config_key(tc, config_hash, "foreign_transform_code", foreign_transform_code, config);
- check_config_key(tc, config_hash, "null_value", null_value, config);
+ /* MVM_string_utf8_decode() can potentially allocate, and hence gc. */
+ MVMROOT(tc, config_hash, {
+ check_config_key(tc, config_hash, "int_box", int_box_type, config);
+ check_config_key(tc, config_hash, "num_box", num_box_type, config);
+ check_config_key(tc, config_hash, "str_box", str_box_type, config);
+ check_config_key(tc, config_hash, "slurpy_array", slurpy_array_type, config);
+ check_config_key(tc, config_hash, "slurpy_hash", slurpy_hash_type, config);
+ check_config_key(tc, config_hash, "array_iter", array_iterator_type, config);
+ check_config_key(tc, config_hash, "hash_iter", hash_iterator_type, config);
+ check_config_key(tc, config_hash, "foreign_type_int", foreign_type_int, config);
+ check_config_key(tc, config_hash, "foreign_type_num", foreign_type_num, config);
+ check_config_key(tc, config_hash, "foreign_type_str", foreign_type_str, config);
+ check_config_key(tc, config_hash, "foreign_transform_array", foreign_transform_array, config);
+ check_config_key(tc, config_hash, "foreign_transform_hash", foreign_transform_hash, config);
+ check_config_key(tc, config_hash, "foreign_transform_code", foreign_transform_code, config);
+ check_config_key(tc, config_hash, "null_value", null_value, config);
+ });
return config_hash;
}
--
1.7.1
