From 74ac5c8dd74d877d1f5a90d0a9706e2ea2f9f8a9 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerberg <39104088+nhz2@users.noreply.github.com> Date: Thu, 20 Jun 2024 17:19:04 -0400 Subject: [PATCH] Add `GC.@preserve` when using pointers (#221) --- docs/src/examples.md | 2 +- src/memory.jl | 1 + src/stream.jl | 28 ++++++++++-------- src/transcode.jl | 15 +++++----- test/codecnoop.jl | 10 ++++--- test/codecquadruple.jl | 3 +- test/runtests.jl | 64 ++++++++++++++++++++++-------------------- 7 files changed, 67 insertions(+), 56 deletions(-) diff --git a/docs/src/examples.md b/docs/src/examples.md index 5b366d81..3644c45a 100644 --- a/docs/src/examples.md +++ b/docs/src/examples.md @@ -178,7 +178,7 @@ using CodecZlib function decompress(input, output) buffer = Vector{UInt8}(undef, 16 * 1024) - while !eof(input) + GC.@preserve buffer while !eof(input) n = min(bytesavailable(input), length(buffer)) unsafe_read(input, pointer(buffer), n) unsafe_write(output, pointer(buffer), n) diff --git a/src/memory.jl b/src/memory.jl index 357df640..81eceb81 100644 --- a/src/memory.jl +++ b/src/memory.jl @@ -11,6 +11,7 @@ struct Memory size::UInt end +# TODO remove this method function Memory(data::ByteData) return Memory(pointer(data), sizeof(data)) end diff --git a/src/stream.jl b/src/stream.jl index 105210ed..c6c5073b 100644 --- a/src/stream.jl +++ b/src/stream.jl @@ -343,16 +343,18 @@ function Base.readuntil(stream::TranscodingStream, delim::UInt8; keep::Bool=fals local ret::Vector{UInt8} filled = 0 while !eof(stream) - p = findbyte(buffer1, delim) - found = false - if p < marginptr(buffer1) - found = true - sz = Int(p + 1 - bufferptr(buffer1)) - if !keep - sz -= 1 + GC.@preserve buffer1 begin + p = findbyte(buffer1, delim) + found = false + if p < marginptr(buffer1) + found = true + sz = Int(p + 1 - bufferptr(buffer1)) + if !keep + sz -= 1 + end + else + sz = buffersize(buffer1) end - else - sz = buffersize(buffer1) end if @isdefined(ret) resize!(ret, filled + sz) @@ -703,9 +705,11 @@ end # Call `process` with prologue and epilogue. function callprocess(stream::TranscodingStream, inbuf::Buffer, outbuf::Buffer) state = stream.state - input = buffermem(inbuf) - GC.@preserve inbuf makemargin!(outbuf, minoutsize(stream.codec, input)) - Δin::Int, Δout::Int, state.code = GC.@preserve inbuf outbuf process(stream.codec, input, marginmem(outbuf), state.error) + makemargin!( + outbuf, + GC.@preserve(inbuf, minoutsize(stream.codec, buffermem(inbuf))), + ) + Δin::Int, Δout::Int, state.code = GC.@preserve inbuf outbuf process(stream.codec, buffermem(inbuf), marginmem(outbuf), state.error) @debug( "called process()", code = state.code, diff --git a/src/transcode.jl b/src/transcode.jl index 32cb5bee..fca33736 100644 --- a/src/transcode.jl +++ b/src/transcode.jl @@ -46,10 +46,10 @@ function Base.transcode(codec::Type{C}, src::String) where {C<:Codec} end _default_output_buffer(codec, input) = Buffer( - initial_output_size( + GC.@preserve(input, initial_output_size( codec, buffermem(input) - ) + )) ) """ @@ -125,8 +125,7 @@ function transcode!( Base.mightalias(input.data, output.data) && error( "input and outbut buffers must be independent" ) - # GC.@preserve since unsafe_transcode! may convert to raw pointers - GC.@preserve input output codec unsafe_transcode!(output, codec, input) + unsafe_transcode!(output, codec, input) end """ @@ -148,10 +147,10 @@ function unsafe_transcode!( if code === :error @goto error end - n = minoutsize(codec, buffermem(input)) + n = GC.@preserve input minoutsize(codec, buffermem(input)) @label process makemargin!(output, n) - Δin, Δout, code = process(codec, buffermem(input), marginmem(output), error) + Δin, Δout, code = GC.@preserve input output process(codec, buffermem(input), marginmem(output), error) @debug( "called process()", code = code, @@ -169,13 +168,13 @@ function unsafe_transcode!( if startproc(codec, :write, error) === :error @goto error end - n = minoutsize(codec, buffermem(input)) + n = GC.@preserve input minoutsize(codec, buffermem(input)) @goto process end resize!(output.data, output.marginpos - 1) return output.data else - n = max(Δout, minoutsize(codec, buffermem(input))) + n = GC.@preserve input max(Δout, minoutsize(codec, buffermem(input))) @goto process end @label error diff --git a/test/codecnoop.jl b/test/codecnoop.jl index 02a6bd48..9db6bc14 100644 --- a/test/codecnoop.jl +++ b/test/codecnoop.jl @@ -24,13 +24,14 @@ using FillArrays: Zeros stream = TranscodingStream(Noop(), IOBuffer()) @test_throws EOFError read(stream, UInt8) - @test_throws EOFError unsafe_read(stream, pointer(Vector{UInt8}(undef, 3)), 3) + data = Vector{UInt8}(undef, 3) + @test_throws EOFError GC.@preserve data unsafe_read(stream, pointer(data), 3) close(stream) stream = TranscodingStream(Noop(), IOBuffer("foobar"), bufsize=1) @test read(stream, UInt8) === UInt8('f') data = Vector{UInt8}(undef, 5) - unsafe_read(stream, pointer(data), 5) === nothing + GC.@preserve data unsafe_read(stream, pointer(data), 5) === nothing @test data == b"oobar" close(stream) @@ -122,7 +123,7 @@ using FillArrays: Zeros stream = TranscodingStream(Noop(), IOBuffer("foo")) out = zeros(UInt8, 3) @test bytesavailable(stream) == 0 - @test TranscodingStreams.unsafe_read(stream, pointer(out), 10) == 3 + @test GC.@preserve out TranscodingStreams.unsafe_read(stream, pointer(out), 10) == 3 @test out == b"foo" close(stream) @@ -384,7 +385,8 @@ using FillArrays: Zeros @test eof(stream) stream = NoopStream(IOBuffer("foobar")) - @test_throws ArgumentError TranscodingStreams.unsafe_unread(stream, pointer(b"foo"), -1) + data = b"foo" + @test_throws ArgumentError GC.@preserve data TranscodingStreams.unsafe_unread(stream, pointer(data), -1) close(stream) stream = NoopStream(IOBuffer("foo")) diff --git a/test/codecquadruple.jl b/test/codecquadruple.jl index 31a0ef53..fe481b67 100644 --- a/test/codecquadruple.jl +++ b/test/codecquadruple.jl @@ -81,7 +81,8 @@ end close(stream2) stream = TranscodingStream(QuadrupleCodec(), IOBuffer("foo")) - @test_throws EOFError unsafe_read(stream, pointer(Vector{UInt8}(undef, 13)), 13) + data = Vector{UInt8}(undef, 13) + @test_throws EOFError GC.@preserve data unsafe_read(stream, pointer(data), 13) close(stream) @testset "position" begin diff --git a/test/runtests.jl b/test/runtests.jl index 310da504..2bca20af 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -26,13 +26,15 @@ using TranscodingStreams: data = Vector{UInt8}(b"foobar") buf = Buffer(data) - @test buf isa Buffer - @test bufferptr(buf) === pointer(data) - @test buffersize(buf) === 6 - @test buffermem(buf) === Memory(pointer(data), 6) - @test marginptr(buf) === pointer(data) + 6 - @test marginsize(buf) === 0 - @test marginmem(buf) === Memory(pointer(data)+6, 0) + GC.@preserve data buf begin + @test buf isa Buffer + @test bufferptr(buf) === pointer(data) + @test buffersize(buf) === 6 + @test buffermem(buf) === Memory(pointer(data), 6) + @test marginptr(buf) === pointer(data) + 6 + @test marginsize(buf) === 0 + @test marginmem(buf) === Memory(pointer(data)+6, 0) + end buf = Buffer(2) writebyte!(buf, 0x34) @@ -72,31 +74,33 @@ end @testset "Memory" begin data = Vector{UInt8}(b"foobar") - mem = TranscodingStreams.Memory(pointer(data), sizeof(data)) - @test mem isa TranscodingStreams.Memory - @test mem.ptr === pointer(data) - @test mem.size === length(mem) === UInt(sizeof(data)) - @test lastindex(mem) === 6 - @test mem[1] === UInt8('f') - @test mem[2] === UInt8('o') - @test mem[3] === UInt8('o') - @test mem[4] === UInt8('b') - @test mem[5] === UInt8('a') - @test mem[6] === UInt8('r') - @test_throws BoundsError mem[7] - @test_throws BoundsError mem[0] - mem[1] = UInt8('z') - @test mem[1] === UInt8('z') - mem[3] = UInt8('!') - @test mem[3] === UInt8('!') - @test_throws BoundsError mem[7] = 0x00 - @test_throws BoundsError mem[0] = 0x00 + GC.@preserve data let mem = TranscodingStreams.Memory(pointer(data), sizeof(data)) + @test mem isa TranscodingStreams.Memory + @test mem.ptr === pointer(data) + @test mem.size === length(mem) === UInt(sizeof(data)) + @test lastindex(mem) === 6 + @test mem[1] === UInt8('f') + @test mem[2] === UInt8('o') + @test mem[3] === UInt8('o') + @test mem[4] === UInt8('b') + @test mem[5] === UInt8('a') + @test mem[6] === UInt8('r') + @test_throws BoundsError mem[7] + @test_throws BoundsError mem[0] + mem[1] = UInt8('z') + @test mem[1] === UInt8('z') + mem[3] = UInt8('!') + @test mem[3] === UInt8('!') + @test_throws BoundsError mem[7] = 0x00 + @test_throws BoundsError mem[0] = 0x00 + end data = Vector{UInt8}(b"foobar") - mem = TranscodingStreams.Memory(data) - @test mem isa TranscodingStreams.Memory - @test mem.ptr == pointer(data) - @test mem.size == sizeof(data) + GC.@preserve data let mem = TranscodingStreams.Memory(pointer(data), sizeof(data)) + @test mem isa TranscodingStreams.Memory + @test mem.ptr == pointer(data) + @test mem.size == sizeof(data) + end end @testset "Stats" begin