New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

me.pool.pull stucks in a loop #443

Closed
samifruit514 opened this Issue Feb 19, 2014 · 9 comments

Comments

Projects
None yet
4 participants
@samifruit514

samifruit514 commented Feb 19, 2014

When I get an entity from the pool, I get an error on the following line:

obj = new (proto.bind.apply(proto, args))();

error:
Uncaught RangeError: Maximum call stack size exceeded

The stack trace is very large but it says it comes from deepcopy

@parasyte

This comment has been minimized.

Show comment
Hide comment
@parasyte

parasyte Feb 19, 2014

Member

This is caused by reference cycles and recursive deepcopy. Without recursion, it would infinite-loop instead.

Member

parasyte commented Feb 19, 2014

This is caused by reference cycles and recursive deepcopy. Without recursion, it would infinite-loop instead.

@obiot

This comment has been minimized.

Show comment
Hide comment
@obiot

obiot Feb 19, 2014

Member

reference cycle in your entity definition most probably i would add !
Looks like more a support question than a bug, so closing this one :)

Member

obiot commented Feb 19, 2014

reference cycle in your entity definition most probably i would add !
Looks like more a support question than a bug, so closing this one :)

@obiot obiot closed this Feb 19, 2014

@parasyte

This comment has been minimized.

Show comment
Hide comment
@parasyte

parasyte Feb 19, 2014

Member

@obiot I argue that this should be fixed by avoiding infinite loops. :) Anyway, I directed him here to create a ticket on this. I'll reopen for further discussion.

Member

parasyte commented Feb 19, 2014

@obiot I argue that this should be fixed by avoiding infinite loops. :) Anyway, I directed him here to create a ticket on this. I'll reopen for further discussion.

@parasyte parasyte reopened this Feb 19, 2014

@obiot

This comment has been minimized.

Show comment
Hide comment
@obiot

obiot Feb 19, 2014

Member

ok sorry i did not know that :)

but to be honest, and if I can give my opinion, I believe that keeping track of all child references to ensure no sub or sub or sub sub (and so on) child refers to any parent references, is maybe a bit complexifying the deepcopy for nothing. And this kind of reference cycle issue (leading to a stack overflow) is somehow something usual for a developer and more a matter of being careful than something else (in my opinion). Not to mention that in both case it will throw an exception (either the browser or then the deepcopy function)

Member

obiot commented Feb 19, 2014

ok sorry i did not know that :)

but to be honest, and if I can give my opinion, I believe that keeping track of all child references to ensure no sub or sub or sub sub (and so on) child refers to any parent references, is maybe a bit complexifying the deepcopy for nothing. And this kind of reference cycle issue (leading to a stack overflow) is somehow something usual for a developer and more a matter of being careful than something else (in my opinion). Not to mention that in both case it will throw an exception (either the browser or then the deepcopy function)

@parasyte

This comment has been minimized.

Show comment
Hide comment
@parasyte

parasyte Feb 19, 2014

Member

So, here's where things get very problematic. A cycle may be created and totally contained in the object itself, without referring to its own parent. In this case, it is safe to deepcopy with cycle protection.

On the other hand, any object in the pool that references a live object could be a problem in user code. However a deepcopy with cycle protection is still safe here.

Third, deepcopy is only used in the constructor (init) which we need to remove from the pooling flow anyway.

Member

parasyte commented Feb 19, 2014

So, here's where things get very problematic. A cycle may be created and totally contained in the object itself, without referring to its own parent. In this case, it is safe to deepcopy with cycle protection.

On the other hand, any object in the pool that references a live object could be a problem in user code. However a deepcopy with cycle protection is still safe here.

Third, deepcopy is only used in the constructor (init) which we need to remove from the pooling flow anyway.

@obiot obiot added this to the 1.2.0 milestone Jul 28, 2014

@obiot

This comment has been minimized.

Show comment
Hide comment
@obiot

obiot Jul 28, 2014

Member

we did not improve anything for the pool system as part of 1.1,0, so moving to 1.2.0

Member

obiot commented Jul 28, 2014

we did not improve anything for the pool system as part of 1.1,0, so moving to 1.2.0

@parasyte

This comment has been minimized.

Show comment
Hide comment
@parasyte

parasyte Jul 28, 2014

Member

This is actually no longer a problem in 1.1.0, because it does not contain deepcopy

Member

parasyte commented Jul 28, 2014

This is actually no longer a problem in 1.1.0, because it does not contain deepcopy

@agmcleod

This comment has been minimized.

Show comment
Hide comment
@agmcleod

agmcleod Jul 28, 2014

Member

Was trying to remember if removed it or not :)

Member

agmcleod commented Jul 28, 2014

Was trying to remember if removed it or not :)

@parasyte parasyte modified the milestones: 1.2.0, 1.1.0 Jul 28, 2014

@parasyte

This comment has been minimized.

Show comment
Hide comment
@parasyte

parasyte Jul 28, 2014

Member

Back to 1.1.0, and closed!

Member

parasyte commented Jul 28, 2014

Back to 1.1.0, and closed!

@parasyte parasyte closed this Jul 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment