Skip to content

Commit

Permalink
[gc] fix a MS segv in Parrot_io_write_s
Browse files Browse the repository at this point in the history
When certain functions cause a GC, the local string buffers may get
moved away, but local STRING* variables or worse, strings inside a
StringHandle may still point to the old location.

Lock the GC then for this section, mostly during IO writing to string handles,
when the handle needs to realloc its buffer (the stringhandle).
But reading into a buffer is also problematic. we really should know the
size beforehand.

One way to fix GH #1196, lock the GC sweep in those cases.
The other variants would be:
- lock the whole GC via Parrot_block_GC_mark
- unset the PObj_is_movable flag in the STRING that it may not be moved,
  as it is still locally referenced.

Remaining ms test failure: t/pmc/fixedstringarray.t
  • Loading branch information
Reini Urban committed Feb 1, 2015
1 parent d56c942 commit a88abb9
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/gc/string_gc.c
@@ -1,5 +1,5 @@
/*
Copyright (C) 2010-2014, Parrot Foundation.
Copyright (C) 2010-2015, Parrot Foundation.
=head1 NAME
Expand Down
8 changes: 6 additions & 2 deletions src/io/api.c
Expand Up @@ -704,6 +704,7 @@ Parrot_io_read_s(PARROT_INTERP, ARGMOD(PMC *handle), size_t length)
const STR_VTABLE * encoding = vtable->get_encoding(interp, handle);
STRING * s;

Parrot_block_GC_sweep(interp); /* Our local strings may not move away. GH #1196 */
/* read_s requires us to read in a whole number of characters, which
might be multi-byte. This requires a read buffer. */
/* TODO: If we have a fixed8 encoding or similar, we should be able to
Expand All @@ -716,6 +717,7 @@ Parrot_io_read_s(PARROT_INTERP, ARGMOD(PMC *handle), size_t length)

s = io_read_encoded_string(interp, handle, vtable, read_buffer, encoding, length);
PARROT_ASSERT(s->strlen <= length);
Parrot_unblock_GC_sweep(interp);
return s;
}
}
Expand Down Expand Up @@ -838,7 +840,7 @@ Parrot_io_read_byte_buffer_pmc(PARROT_INTERP, ARGMOD(PMC *handle),

{
const IO_VTABLE * const vtable = IO_GET_VTABLE(interp, handle);
IO_BUFFER * const read_buffer = IO_GET_READ_BUFFER(interp, handle);
IO_BUFFER * const read_buffer = IO_GET_READ_BUFFER(interp, handle);
IO_BUFFER * const write_buffer = IO_GET_WRITE_BUFFER(interp, handle);
size_t bytes_read;
char * content;
Expand Down Expand Up @@ -1065,12 +1067,14 @@ Parrot_io_write_s(PARROT_INTERP, ARGMOD(PMC *handle), ARGIN(STRING *s))
size_t bytes_written;

io_verify_is_open_for(interp, handle, vtable, PIO_F_WRITE);

Parrot_block_GC_sweep(interp); /* Our local string may not move away. GH #1196 */
io_sync_buffers_for_write(interp, handle, vtable, read_buffer, write_buffer);
out_s = io_verify_string_encoding(interp, handle, vtable, s, PIO_F_WRITE);

bytes_written = Parrot_io_buffer_write_b(interp, write_buffer, handle,
vtable, out_s->strstart, out_s->bufused);
vtable->adv_position(interp, handle, bytes_written);
Parrot_unblock_GC_sweep(interp);

/* If we are writing to a r/w handle, advance the pointer in the
associated read-buffer since we're overwriting those characters. */
Expand Down

0 comments on commit a88abb9

Please sign in to comment.