omniscientjs/immstruct

Maximum call stack size exceeded

Closed this issue · 7 comments

Hi,
here is a little sample that crash the call stack in the current 1.4.0 and the master one.
I want to swap 2 element in an List and modify their content.

var structure = Immstruct('a', [ {a:1}, {b:2}]);
var cursor = structure.cursor();
var ca = cursor.get(0);
var res  = cursor.withMutations(function(c){
      c.set(0, cursor.get(1))
      .set(1, ca)});
structure.cursor().setIn([0, 'b'], 3)

This raise a stack overflow error.
Any idea ?
Thanks.

There might be that there is a circular structure dependency happening here in immstructs update function (merging). I checked the equivalent with pure Immutable.js (without cursors), and it seemed to work. The same goes for "vanilla" cursors.

As I suspected, there is a circular dependency between a and b caused by something in the immstruct code and perhaps persistant data structures.

Doing something like this would work:

var structure = immstruct([ {a:1}, {b:2}]);
var cursor = structure.cursor();
var ca = cursor.get(0).deref();

var res  = cursor.withMutations(function(c){
  c.set(0, cursor.get(1).deref())
   .set(1, ca)
});

structure.cursor().setIn([0, 'a'], 3);

Need to dive more into this to find where the actual stack overflow occurs.

Yes, it works now, so did I misused the api and should have .deref() or is it a bug in the lib ?
Thanks.

It's a bug in the library that either refers to old structure and as the places has swapped it causes circular references, or there is something else that causes it.

It might actually be related to #45 if we're "lucky" (edit, probably not, misremembered the issue)

@hokkos cursor.get(0) and cursor.get(1) return Cursor instances; not Immutable.Map:

    /**
     * Returns the value at the `key` in the cursor, or `notSetValue` if it
     * does not exist.
     *
     * If the key would return a collection, a new Cursor is returned.
     */
    get(key: any, notSetValue?: any): any;

Ah, of course, @dashed. I didn't quite make sense of it while debugging right now. So yeah, by swapping the two data points with their cursors, you get two references which causes the circular dependency between them self and causing the infinite indirect recursion by Immutable.js cursors implementation.

@hokkos So, this is a usage error, not a bug. Didn't think about Cursor#get returning a Cursor. You want to update to insert the data structure not the actual cursors, so doing deref as I showed you would be correct.

Thanks.