I believe that MVM_file_set_encoding() needs to root oshandle (patch attached) as MVM_string_find_encoding() can allocate and hence can cause a GC run.
There are two other uses of MVM_string_find_encoding(). One is: /* At least find_encoding may allocate on first call, so root just * in case. */ MVMROOT(tc, buf, { encoded = MVM_string_encode(tc, s, 0, NUM_GRAPHS(s), &output_size, MVM_string_find_encoding(tc, enc_name)); }); The other is: /* Decode. */ return MVM_string_decode(tc, tc->instance->VMString, ((MVMArray *)buf)->body.slots.i8 + ((MVMArray *)buf)->body.start, ((MVMArray *)buf)->body.elems * elem_size, MVM_string_find_encoding(tc, enc_name)); Is this one buggy, because tc->instance->VMString is a GC-able object, and so might move if MVM_string_find_encoding() triggers a GC collection? ie, should that MVM_string_find_encoding(tc, enc_name) be yanked to a line above and assigned to a local variable, so that any GC run has happened before the arguments to MVM_string_decode() start to be put onto the CPU stack for the C function call? Nicholas Clark
>From b54de7c404e5bd0121f13be811e638581374b0ad Mon Sep 17 00:00:00 2001 From: Nicholas Clark <n...@ccl4.org> Date: Sat, 28 Dec 2013 16:58:45 +0100 Subject: [PATCH] MVM_file_set_encoding() needs to root oshandle MVM_string_find_encoding() can allocate, and hence can cause a GC run. --- src/io/fileops.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/io/fileops.c b/src/io/fileops.c index 870fb84..62b9f83 100644 --- a/src/io/fileops.c +++ b/src/io/fileops.c @@ -834,9 +834,10 @@ MVMint64 MVM_file_eof(MVMThreadContext *tc, MVMObject *oshandle) { void MVM_file_set_encoding(MVMThreadContext *tc, MVMObject *oshandle, MVMString *encoding_name) { MVMOSHandle *handle; - const MVMuint8 encoding_flag = MVM_string_find_encoding(tc, encoding_name); + MVMROOT(tc, oshandle, { + const MVMuint8 encoding_flag = MVM_string_find_encoding(tc, encoding_name); - verify_filehandle_type(tc, oshandle, &handle, "setencoding"); - - handle->body.encoding_type = encoding_flag; + verify_filehandle_type(tc, oshandle, &handle, "setencoding"); + handle->body.encoding_type = encoding_flag; + }); } -- 1.7.1