Opened 12 years ago

Closed 11 years ago

#2486 closed defect (wontfix)

dojo.clone does not recognize cycles in object graphs

Reported by: valeri.felberg@… Owned by: James Burke
Priority: high Milestone: future
Component: Core Version: 0.4.1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

Following code causes an infinite recursion and crashes the browser (tested in Firefox 2 and IE 6):

var x = {}
var y = {};
    
x.y = y;
y.x = x;
    
var z = dojo.clone(x, true);

If you omit the deep parameter or set it to false, the browser does not crash but if you call dojo.debugDeep(z), Firefox shows an apparently infinite chain of objects (broken after some limit) and IE crashes.

Attachments (1)

clone.html (472 bytes) - added by dante 11 years ago.
still an issue. too much recursion in lang.js line 216

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 years ago by dylan

Milestone: 1.2
Owner: changed from anonymous to dante

Please test this with clone and see if it is still an issue.

Changed 11 years ago by dante

Attachment: clone.html added

still an issue. too much recursion in lang.js line 216

comment:2 Changed 11 years ago by dante

Description: modified (diff)
Milestone: 1.21.4

comment:3 Changed 11 years ago by bill

Component: GeneralCore
Description: modified (diff)
Summary: dojo.lang.shallowCopy does not recognize cycles in object graphsdojo.clone does not recognize cycles in object graphs

comment:4 Changed 11 years ago by bill

Owner: changed from dante to James Burke

I assume that we don't want to fix this? I feel like we've had this discussion before.

comment:5 Changed 11 years ago by Eugene Lazutkin

Do you realize that this ticket is for 0.4.1? We did have this discussion back in 0.9, and decided:

  • If you want a shallow copy of an object y = dojo.mixin({}, x);
  • If you want a shallow copy of an array: y = dojo.map(x, "return value;");
  • The rest will be covered by the deep copy: y = dojo.clone(x);

That's why we don't support deep argument anymore --- dojo.clone() is always deep. Cyclic (e.g., circular or DAG) cases are explicitly not supported (see the inlined documents) due to speed and space concerns. Such advanced version is seldom needed, and should go to DojoX or Core, but clearly it should be out of the Base (where dojo.clone() is).

I propose to close this ticket as obsolete/wontfix, and open a new enhancement ticket for the advanced version.

If somebody wants to do the advanced version, please contact me --- I have some additional requirements for it.

comment:6 Changed 11 years ago by dante

Milestone: 1.4future

I added this explanation to http://docs.dojocampus.org/dojo/clone (which didn't exist). feel free to add further examples in the docs of its usage. +1 on wontfix

comment:7 Changed 11 years ago by bill

Resolution: wontfix
Status: newclosed

Marking as wont-fix (+1 from me too. I assume we don't need an official foundation vote :-) )

Note: See TracTickets for help on using tickets.