identity preservation for custom serialized structs (#432)

* identity preservation for custom serialized structs * fix #433 circular references with custom serialization * differentiate between mutable and immutable * exception for Core.SimpleVector * version readme and changelog Co-authored-by: Jonas Isensee <[email protected]>
JuliaIO · Nov 7, 2022 · 00e1931 · 00e1931 · JonasIsensee · Nov 7, 2022
1 parent 0bc6aec
commit 00e1931
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 0.4.26
+ - fix identity relations with custom serialization
+
 ## 0.4.25
  - remove leftover debug statement
 

diff --git a/Project.toml b/Project.toml
@@ -1,6 +1,6 @@
 name = "JLD2"
 uuid = "033835bb-8acc-5ee8-8aae-3f567f8a3819"
-version = "0.4.25"
+version = "0.4.26"
 
 [deps]
 FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"

diff --git a/README.md b/README.md
@@ -4,7 +4,8 @@
 |:--------:|:---------------:|:-------:|:---:|
 |[![](https://img.shields.io/badge/docs-online-blue.svg)](https://JuliaIO.github.io/JLD2.jl/dev)| [![CI](https://github.com/juliaio/JLD2.jl/workflows/CI/badge.svg?branch=master)](https://github.com/JuliaIO/JLD2.jl/actions) | [![codecov.io](https://codecov.io/github/JuliaIO/JLD2.jl/coverage.svg?branch=master)](https://codecov.io/github/JuliaIO/JLD2.jl?branch=master) | [![JLD2 Downloads](https://shields.io/endpoint?url=https://pkgs.genieframework.com/api/v1/badge/JLD2)](https://pkgs.genieframework.com?packages=JLD2) |
 
-JLD2 saves and loads Julia data structures in a format comprising a subset of HDF5, without any dependency on the HDF5 C library. It typically outperforms [the JLD package](https://github.com/JuliaIO/JLD.jl) (sometimes by multiple orders of magnitude) and often outperforms Julia's built-in serializer. While other HDF5 implementations supporting HDF5 File Format Specification Version 3.0 (i.e. libhdf5 1.10 or later) should be able to read the files that JLD2 produces, JLD2 is likely to be incapable of reading files created or modified by other HDF5 implementations. JLD2 does not aim to be backwards or forwards compatible with the JLD package.
+JLD2 saves and loads Julia data structures in a format comprising a subset of HDF5, without any dependency on the HDF5 C library. 
+JLD2 is able to read most HDF5 files created by other HDF5 implementations supporting HDF5 File Format Specification Version 3.0 (i.e. libhdf5 1.10 or later) and similarly those should be able to read the files that JLD2 produces. JLD2 provides read-only support for files created with the JLD package.
 
 ## Reading and writing data
 ### `save` and `load` functions

diff --git a/src/data/custom_serialization.jl b/src/data/custom_serialization.jl
@@ -42,6 +42,28 @@ jlconvert_canbeuninitialized(::ReadRepresentation{T,CustomSerialization{S,ODR}})
     jlconvert_canbeuninitialized(ODR)
 jlconvert_isinitialized(::ReadRepresentation{T,CustomSerialization{S,ODR}}, ptr::Ptr) where {T,S,ODR} =
     jlconvert_isinitialized(ReadRepresentation{S,ODR}(), ptr)
-jlconvert(::ReadRepresentation{T,CustomSerialization{S,ODR}},
-          f::JLDFile, ptr::Ptr, header_offset::RelOffset) where {T,S,ODR} =
-    rconvert(T, jlconvert(ReadRepresentation{S,ODR}(), f, ptr, header_offset))::T
+
+
+function jlconvert(::ReadRepresentation{T,CustomSerialization{S,ODR}},
+          f::JLDFile, ptr::Ptr, header_offset::RelOffset) where {T,S,ODR}
+
+    if ismutabletype(T) && !(T <: Core.SimpleVector)
+        # May encounter a self-referential struct that used custom serialization
+        # provide an unitialized struct and later fill it with values
+        obj = newstruct(T)
+        track_weakref!(f, header_offset, obj)
+
+        # actually load the data
+        v = rconvert(T, jlconvert(ReadRepresentation{S,ODR}(), f, ptr, header_offset))::T
+        # copy fields to initial struct
+        for i in 0:nfields(obj)-1
+            fieldval = ccall(:jl_get_nth_field, Any, (Any, Csize_t), v, i)
+            ccall(:jl_set_nth_field, Nothing, (Any, Csize_t, Any), obj, i, fieldval)
+        end
+        return obj
+    else
+        v = rconvert(T, jlconvert(ReadRepresentation{S,ODR}(), f, ptr, header_offset))::T
+        track_weakref!(f, header_offset, v)
+        return v
+    end
+end
diff --git a/src/data/reconstructing_datatypes.jl b/src/data/reconstructing_datatypes.jl
@@ -537,7 +537,7 @@ jlconvert(::ReadRepresentation{Core.TypeofBottom,nothing}, f::JLDFile, ptr::Ptr,
     if ismutabletype(T)
         push!(args, quote
             obj = $(Expr(:new, T))
-            track_weakref!(f, header_offset, obj)
+            track_weakref_if_untracked!(f, header_offset, obj)
         end)
         fn = fieldnames(T)
         for i = 1:length(types)

diff --git a/src/data/writing_datatypes.jl b/src/data/writing_datatypes.jl
@@ -8,6 +8,15 @@ function track_weakref!(f::JLDFile, header_offset::RelOffset, @nospecialize v)
     nothing
 end
 
+function track_weakref_if_untracked!(f::JLDFile, header_offset::RelOffset, @nospecialize v)
+    if header_offset !== NULL_REFERENCE 
+        if !haskey(f.jloffset, header_offset)
+            f.jloffset[header_offset] = WeakRef(v)
+        end
+    end
+    nothing
+end
+
 ## Generic machinery
 
 # Carries the type and on-disk representation of data to be read from
@@ -305,6 +314,7 @@ jlconvert_canbeuninitialized(::ReadRepresentation{RelOffset,RelOffset}) = false
     x = load_dataset(f, jlunsafe_load(convert(Ptr{RelOffset}, ptr)))
     (isa(x, T) ? x : rconvert(T, x))::T
 end
+
 jlconvert_canbeuninitialized(::ReadRepresentation{T,RelOffset}) where {T} = true
 jlconvert_isinitialized(::ReadRepresentation{T,RelOffset}, ptr::Ptr) where {T} =
     jlunsafe_load(convert(Ptr{RelOffset}, ptr)) != NULL_REFERENCE

diff --git a/test/loadsave.jl b/test/loadsave.jl
@@ -540,4 +540,62 @@ if VERSION ≥ v"1.8"
         end
     end
 
+end
+
+## Issue #431 Identity-Preservation of nested structs with CustomSerialization 
+
+abstract type AT end
+Base.@kwdef mutable struct T1 <: AT
+    f
+end
+Base.@kwdef mutable struct T2 <: AT
+    t1::T1
+end
+Base.@kwdef mutable struct T3 <: AT
+    t1::T1
+    t2::T2
+end
+
+DSA=Dict{Symbol, Any}
+JLD2.writeas(::Type{T}) where {T <: AT} = DSA
+JLD2.wconvert(::Type{DSA}, t::AT) = DSA(f => getproperty(t, f) for f in fieldnames(typeof(t)))
+JLD2.rconvert(::Type{T}, dsa::DSA) where {T <: AT} = T(; dsa...)
+
+@testset "Issue #431 Identity-Preservation" begin
+    cd(mktempdir()) do
+        t1 = T1(1)
+        t2 = T2(t1)
+        t3 = T3(t1, t2)
+        save_object("kk.jld2", t3)
+
+        t3 = load_object("kk.jld2")
+        @test t3.t1 === t3.t2.t1
+    end
+end
+
+## Issue #433 circular references with custom serialization
+
+
+mutable struct CR
+    r::CR
+    CR() = new()
+    CR(x) = new(x)
+end
+
+mutable struct CRSerialized
+    r::CR
+end
+
+JLD2.writeas(::Type{CR}) = CRSerialized
+JLD2.wconvert(::Type{CRSerialized}, t::CR) = CRSerialized(t.r)
+JLD2.rconvert(::Type{CR}, dsa::CRSerialized) = CR(dsa.r)
+@testset "Issue #433 circular references with custom serialization" begin
+    cd(mktempdir()) do
+        cr = CR()
+        cr.r = cr
+        save_object("kk.jld2", cr)
+
+        t1 = load_object("kk.jld2")
+        @test t1 === t1.r
+    end
 end