Skip to content

Commit 6fdede0

Browse files
hsbtclaude
andcommitted
Free tag-directive buffers on the error path in start_document
The xcalloc'd tag buffers leaked when StringValue, the tuple-length check, or the emit raised mid-way. Wrap the work in rb_ensure so the buffers are always freed, and keep the exported directive strings in a Ruby array so the GC cannot reclaim them while their C pointers are in use. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4bd1343 commit 6fdede0

1 file changed

Lines changed: 56 additions & 24 deletions

File tree

ext/psych/psych_emitter_fy.c

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,27 @@ static VALUE end_stream(VALUE self)
150150
return self;
151151
}
152152

153-
static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
153+
struct start_document_data {
154+
VALUE self;
155+
VALUE version;
156+
VALUE tags;
157+
VALUE imp;
158+
struct fy_tag *tag_storage;
159+
const struct fy_tag **tag_ptrs;
160+
};
161+
162+
static VALUE start_document_try(VALUE d)
154163
{
164+
struct start_document_data *data = (struct start_document_data *)d;
165+
VALUE version = data->version;
166+
VALUE tags = data->tags;
155167
psych_fy_emitter_t *e;
156168
struct fy_version ver;
157169
const struct fy_version *verp = NULL;
158-
struct fy_tag *tag_storage = NULL;
159-
const struct fy_tag **tag_ptrs = NULL;
160-
VALUE *exported = NULL;
161-
long len = 0;
170+
VALUE guard = Qnil;
171+
struct fy_event *event;
162172

163-
TypedData_Get_Struct(self, psych_fy_emitter_t, &psych_emitter_type, e);
173+
TypedData_Get_Struct(data->self, psych_fy_emitter_t, &psych_emitter_type, e);
164174
Check_Type(version, T_ARRAY);
165175

166176
if (RARRAY_LEN(version) >= 2) {
@@ -171,19 +181,20 @@ static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
171181

172182
if (RTEST(tags)) {
173183
rb_encoding *encoding = rb_utf8_encoding();
184+
long i, len;
174185
Check_Type(tags, T_ARRAY);
175186
len = RARRAY_LEN(tags);
176187
if (len > 0) {
177-
long i;
178-
tag_storage = xcalloc((size_t)len, sizeof(struct fy_tag));
179-
tag_ptrs = xcalloc((size_t)len + 1, sizeof(struct fy_tag *));
180-
exported = xcalloc((size_t)len * 2, sizeof(VALUE));
188+
/* Ruby array keeps the exported strings reachable for the GC while
189+
* their C pointers live in tag_storage. */
190+
guard = rb_ary_new_capa(len * 2);
191+
data->tag_storage = xcalloc((size_t)len, sizeof(struct fy_tag));
192+
data->tag_ptrs = xcalloc((size_t)len + 1, sizeof(struct fy_tag *));
181193
for (i = 0; i < len; i++) {
182194
VALUE tuple = RARRAY_AREF(tags, i);
183195
VALUE name, value;
184196
Check_Type(tuple, T_ARRAY);
185197
if (RARRAY_LEN(tuple) < 2) {
186-
xfree(tag_storage); xfree(tag_ptrs); xfree(exported);
187198
rb_raise(rb_eRuntimeError, "tag tuple must be of length 2");
188199
}
189200
name = RARRAY_AREF(tuple, 0);
@@ -192,27 +203,48 @@ static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
192203
StringValue(value);
193204
name = rb_str_export_to_enc(name, encoding);
194205
value = rb_str_export_to_enc(value, encoding);
195-
exported[i * 2] = name;
196-
exported[i * 2 + 1] = value;
197-
tag_storage[i].handle = StringValueCStr(name);
198-
tag_storage[i].prefix = StringValueCStr(value);
199-
tag_ptrs[i] = &tag_storage[i];
206+
rb_ary_push(guard, name);
207+
rb_ary_push(guard, value);
208+
data->tag_storage[i].handle = StringValueCStr(name);
209+
data->tag_storage[i].prefix = StringValueCStr(value);
210+
data->tag_ptrs[i] = &data->tag_storage[i];
200211
}
201-
tag_ptrs[len] = NULL;
212+
data->tag_ptrs[len] = NULL;
202213
}
203214
}
204215

205-
struct fy_event *event = fy_emit_event_create(e->emit, FYET_DOCUMENT_START,
206-
imp ? 1 : 0, verp, tag_ptrs);
216+
event = fy_emit_event_create(e->emit, FYET_DOCUMENT_START,
217+
data->imp ? 1 : 0, verp, data->tag_ptrs);
207218

208-
if (exported) { (void)exported[0]; }
209219
do_emit(e, event);
220+
RB_GC_GUARD(guard);
210221

211-
if (tag_storage) xfree(tag_storage);
212-
if (tag_ptrs) xfree(tag_ptrs);
213-
if (exported) xfree(exported);
222+
return data->self;
223+
}
214224

215-
return self;
225+
static VALUE start_document_ensure(VALUE d)
226+
{
227+
struct start_document_data *data = (struct start_document_data *)d;
228+
229+
xfree(data->tag_storage);
230+
xfree(data->tag_ptrs);
231+
232+
return Qnil;
233+
}
234+
235+
static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
236+
{
237+
struct start_document_data data = {
238+
.self = self,
239+
.version = version,
240+
.tags = tags,
241+
.imp = imp,
242+
.tag_storage = NULL,
243+
.tag_ptrs = NULL,
244+
};
245+
246+
return rb_ensure(start_document_try, (VALUE)&data,
247+
start_document_ensure, (VALUE)&data);
216248
}
217249

218250
static VALUE end_document(VALUE self, VALUE imp)

0 commit comments

Comments
 (0)