EXCEPTION_ACCESS_VIOLATION on Windows

[joshday]

I've run into an EXCEPTION_ACCESS_VIOLATION that I can reproduce on Windows 10, but not my Mac. I'm unfortunately unable to share the file/code to reproduce it.

The problem is connected to unsafe_wrap (discovered through the assistance of @JeffBezanson) and I was able to get past the error by changing the keyword arg to own=false in the two functions below:

HDF5.jl/src/HDF5.jl

Lines 1541 to 1546 in 1d74013

	function p2a(p::Ptr{T}, len::Int) where {T<:HDF5Scalar}
	unsafe_wrap(Array, p, len, own=true)
	end
	function p2a(p::Ptr{C}, len::Int) where {C<:CharType}
	stringtype(C)(unsafe_wrap(Array, convert(Ptr{UInt8}, p), len, own=true))
	end

Does own=false likely cause a memory leak here?

[JeffBezanson]

I've been trying to read the libhdf5 docs, but it doesn't seem to make totally clear how this memory should be freed. Are we expected to call free, or is the memory freed when the file is closed, or something else? If we are supposed to call free, then on windows we might need to set up a finalizer that calls the free that goes with the malloc libhdf5 is using instead of what julia is using.

[kleinhenz]

I think that this memory is meant to be freed by calling H5D_VLEN_RECLAIM so it looks to me like own=true is probably wrong.

[kleinhenz]

In the code for handling compound types I used

normalize_types(x::VariableArray) = copy(unsafe_wrap(Array, convert(Ptr{eltype(x)}, x.p), x.len, own=false))

so that the julia array is completely separate from the hdf5 allocated memory and then I call the reclaim function. I guess it should be done like this for all vlen types.

[simonbyrne]

Apparently it isn't safe to call free across shared library boundaries on Windows:
JuliaLang/julia#7906 (comment)

[musm]

Is there anything we can do in that case to fix this ?

[simonbyrne]

The options are either to get HDF5 to use the Julia Libc.malloc, or just copy the data using unsafe_string (which is probably the easier option).

[musm]

@kleinhenz any chance you would like to open a PR with that change ? Seems sensible to make that change here