Handle circular refs when converting to Perl

Also check for I32 overflow when converting Vector to AV.

Second half of CLOWNFISH-36.


Project: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/commit/d9829fb0
Tree: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/tree/d9829fb0
Diff: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/diff/d9829fb0

Branch: refs/heads/master
Commit: d9829fb0dfdfbedc39463d19e2d1958d9a84593d
Parents: f3eea9b
Author: Nick Wellnhofer <wellnho...@aevum.de>
Authored: Tue Mar 8 11:54:53 2016 +0100
Committer: Nick Wellnhofer <wellnho...@aevum.de>
Committed: Thu Mar 10 14:47:23 2016 +0100

----------------------------------------------------------------------
 runtime/perl/t/binding/016-vector.t |  5 +-
 runtime/perl/t/binding/017-hash.t   | 15 ++++--
 runtime/perl/xs/XSBind.c            | 83 ++++++++++++++++++++++++++++++--
 3 files changed, 95 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/d9829fb0/runtime/perl/t/binding/016-vector.t
----------------------------------------------------------------------
diff --git a/runtime/perl/t/binding/016-vector.t 
b/runtime/perl/t/binding/016-vector.t
index 15c3942..7f07a53 100644
--- a/runtime/perl/t/binding/016-vector.t
+++ b/runtime/perl/t/binding/016-vector.t
@@ -16,7 +16,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 3;
+use Test::More tests => 4;
 use Clownfish qw( to_clownfish );
 
 my ( $vector, $twin );
@@ -57,3 +57,6 @@ is(
     'to_clownfish($arrayref) handles deep circular references'
 );
 
+my $roundtripped = $vector->to_perl;
+is_deeply( $roundtripped, $arrayref, 'to_perl handles circular references');
+

http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/d9829fb0/runtime/perl/t/binding/017-hash.t
----------------------------------------------------------------------
diff --git a/runtime/perl/t/binding/017-hash.t 
b/runtime/perl/t/binding/017-hash.t
index b69d8e7..f2aefa7 100644
--- a/runtime/perl/t/binding/017-hash.t
+++ b/runtime/perl/t/binding/017-hash.t
@@ -16,7 +16,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 12;
+use Test::More tests => 14;
 use Clownfish qw( to_clownfish );
 
 my $hash = Clownfish::Hash->new( capacity => 10 );
@@ -53,7 +53,16 @@ $hash = to_clownfish($hashref);
 is( $$hash, ${ $hash->fetch_raw('foo') },
     'to_clownfish($hashref) handles circular references' );
 
-$hash = to_clownfish({ key => $hashref })->fetch_raw('key');
-is( $$hash, ${ $hash->fetch_raw('bar')->fetch_raw(0) },
+my $roundtripped = $hash->to_perl;
+is_deeply( $roundtripped, $hashref, 'to_perl handles circular references' );
+
+$hashref = { key => $hashref };
+$hash = to_clownfish($hashref);
+my $val = $hash->fetch_raw('key');
+is( $$val, ${ $val->fetch_raw('bar')->fetch_raw(0) },
     'to_clownfish($hashref) handles deep circular references' );
 
+$roundtripped = $hash->to_perl;
+is_deeply( $roundtripped, $hashref,
+           'to_perl handles deep circular references' );
+

http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/d9829fb0/runtime/perl/xs/XSBind.c
----------------------------------------------------------------------
diff --git a/runtime/perl/xs/XSBind.c b/runtime/perl/xs/XSBind.c
index 793d5eb..1fdb836 100644
--- a/runtime/perl/xs/XSBind.c
+++ b/runtime/perl/xs/XSBind.c
@@ -987,25 +987,64 @@ CFISH_BB_To_Host_IMP(cfish_ByteBuf *self, void *vcache) {
 void*
 CFISH_Vec_To_Host_IMP(cfish_Vector *self, void *vcache) {
     dTHX;
+    cfish_ConversionCache *cache = (cfish_ConversionCache*)vcache;
+    cfish_ConversionCache  new_cache;
+
+    if (cache) {
+        // Lookup Vector in conversion cache.
+        if ((cfish_Obj*)self == cache->root_obj) {
+            return newRV_inc(cache->root_sv);
+        }
+        if (cache->seen) {
+            void *cached_av = CFISH_PtrHash_Fetch(cache->seen, self);
+            if (cached_av) {
+                return newRV_inc((SV*)cached_av);
+            }
+        }
+    }
+
     AV *perl_array = newAV();
-    uint32_t num_elems = CFISH_Vec_Get_Size(self);
+
+    if (!cache) {
+        // Set up conversion cache.
+        cache = &new_cache;
+        cache->root_obj = (cfish_Obj*)self;
+        cache->root_sv  = (SV*)perl_array;
+        cache->seen     = NULL;
+    }
+    else {
+        if (!cache->seen) {
+            // Create PtrHash lazily.
+            cache->seen = cfish_PtrHash_new(0);
+        }
+        CFISH_PtrHash_Store(cache->seen, self, perl_array);
+    }
+
+    size_t num_elems = CFISH_Vec_Get_Size(self);
 
     // Iterate over array elems.
     if (num_elems) {
+        if (num_elems > I32_MAX) {
+            THROW(CFISH_ERR, "Vector too large for Perl AV");
+        }
         av_fill(perl_array, num_elems - 1);
-        for (uint32_t i = 0; i < num_elems; i++) {
+        for (size_t i = 0; i < num_elems; i++) {
             cfish_Obj *val = CFISH_Vec_Fetch(self, i);
             if (val == NULL) {
                 continue;
             }
             else {
                 // Recurse for each value.
-                SV *const val_sv = (SV*)CFISH_Obj_To_Host(val, vcache);
+                SV *const val_sv = (SV*)CFISH_Obj_To_Host(val, cache);
                 av_store(perl_array, i, val_sv);
             }
         }
     }
 
+    if (cache == &new_cache && cache->seen) {
+        CFISH_PtrHash_Destroy(cache->seen);
+    }
+
     return newRV_noinc((SV*)perl_array);
 }
 
@@ -1014,7 +1053,39 @@ CFISH_Vec_To_Host_IMP(cfish_Vector *self, void *vcache) {
 void*
 CFISH_Hash_To_Host_IMP(cfish_Hash *self, void *vcache) {
     dTHX;
+    cfish_ConversionCache *cache = (cfish_ConversionCache*)vcache;
+    cfish_ConversionCache  new_cache;
+
+    if (cache) {
+        // Lookup Hash in conversion cache.
+        if ((cfish_Obj*)self == cache->root_obj) {
+            return newRV_inc(cache->root_sv);
+        }
+        if (cache->seen) {
+            void *cached_hv = CFISH_PtrHash_Fetch(cache->seen, self);
+            if (cached_hv) {
+                return newRV_inc((SV*)cached_hv);
+            }
+        }
+    }
+
     HV *perl_hash = newHV();
+
+    if (!cache) {
+        // Set up conversion cache.
+        cache = &new_cache;
+        cache->root_obj = (cfish_Obj*)self;
+        cache->root_sv  = (SV*)perl_hash;
+        cache->seen     = NULL;
+    }
+    else {
+        if (!cache->seen) {
+            // Create PtrHash lazily.
+            cache->seen = cfish_PtrHash_new(0);
+        }
+        CFISH_PtrHash_Store(cache->seen, self, perl_hash);
+    }
+
     cfish_HashIterator *iter = cfish_HashIter_new(self);
 
     // Iterate over key-value pairs.
@@ -1026,7 +1097,7 @@ CFISH_Hash_To_Host_IMP(cfish_Hash *self, void *vcache) {
         // Recurse for each value.
         cfish_Obj *val    = CFISH_HashIter_Get_Value(iter);
         SV        *val_sv = val
-                            ? (SV*)CFISH_Obj_To_Host(val, vcache)
+                            ? (SV*)CFISH_Obj_To_Host(val, cache)
                             : newSV(0);
 
         // Using a negative `klen` argument to signal UTF-8 is undocumented
@@ -1034,6 +1105,10 @@ CFISH_Hash_To_Host_IMP(cfish_Hash *self, void *vcache) {
         hv_store(perl_hash, key_ptr, -key_size, val_sv, 0);
     }
 
+    if (cache == &new_cache && cache->seen) {
+        CFISH_PtrHash_Destroy(cache->seen);
+    }
+
     CFISH_DECREF(iter);
     return newRV_noinc((SV*)perl_hash);
 }

Reply via email to