Skip to content
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

Infinite recursion on stringification of objects with circular references #8113

Conversation

RealyUniqueName
Copy link
Member

@RealyUniqueName RealyUniqueName commented Apr 8, 2019

Fixes #7903.
PR to see which targets have the same issue.

Recursive objects:

  • java
  • c#
  • as3
  • flash
  • cpp

Recursive arrays:

  • neko
  • hl
  • java
  • cs
  • cpp

@Simn Simn added this to the Release 4.0 milestone Apr 10, 2019
@Simn
Copy link
Member

Simn commented Apr 10, 2019

This would be good to fix for 4.0 because it frequently causes test confusion.

@Simn Simn self-assigned this Apr 10, 2019
@Simn
Copy link
Member

Simn commented Apr 10, 2019

@hughsando: Could you fix this for hxcpp? Here's a standalone test case:

class Main {
	static public function main() {
		var o = {rec: (null : Dynamic)};
		o.rec = o;
		trace(Std.string(o));
	}
}

@Simn
Copy link
Member

Simn commented Apr 10, 2019

Uhm, this fails on flash/as3 too.

@Simn
Copy link
Member

Simn commented Apr 15, 2019

While we're at it, we should also check printing of recursive enum structures.

@RealyUniqueName
Copy link
Member Author

Added a test for recursive enums

@RealyUniqueName
Copy link
Member Author

These targets fail recursive array check: neko, hl, java, cs, cpp (added a checklist to the first post)

@ncannasse
Copy link
Member

I'm not sure I'm willing to fix this in HL . Given toString() can be user implemented, it's quite easy for the user to create a stack overflow with simultaneous calls. For instance:

class A {
    public var me : A;
    public function new() { me = this; };
    public function toString() {
       return "A"+me.toString();
    }
   static function main() {
        trace(new A());
   }
}

I'm not sure why Array should be a specific case.

@RealyUniqueName
Copy link
Member Author

I understand that. But as a user, I've faced frustrating situations when I've got infinite recursion without any custom toString involved.
And as @Simn said we experience this issue in our test suite from time to time.

@ncannasse
Copy link
Member

Well, HL have a VScode integrated debugger and the stack overflow should be correctly reported and catch. We want of course to avoid crash or eating all the memory, but I'm not sure how to test that exactly.

@Simn
Copy link
Member

Simn commented Apr 16, 2019

Not sure why the fact that users can cause this would be an argument against fixing it in std.

@hughsando
Copy link
Member

Yeah, this has got me many times over the years with many targets.
What do you think is the best way of handling this?

  1. Limit the total number of chars output - might also be useful in the case of a huge array
  2. Detect repeated structures (by pointer/reference) and print "..." or something
  3. Limit depth of recursion
  4. Do whatever is easiest
    We could add a common StringTools.toPrettyString(Dyanmic,options) to help with some of these

@Simn
Copy link
Member

Simn commented Apr 17, 2019

On other targets we limit the depth of recursion, so I think hxcpp should do that as well.

@terurou
Copy link
Member

terurou commented May 22, 2019

Given toString() can be user implemented, it's quite easy for the user to create a stack overflow with simultaneous calls.

This problem is reproduced in JS target.
http://try-haxe.mrcdk.com/#eBE77

@Simn
Copy link
Member

Simn commented May 29, 2019

@RealyUniqueName Could you merge development again so we can see where we're at here?

@Simn Simn assigned Aurel300 and unassigned Simn and hughsando May 29, 2019
@Simn
Copy link
Member

Simn commented May 29, 2019

@Aurel300 Could you check the hxcpp hang? I think it's the last thing to address here.

@Aurel300
Copy link
Member

Aurel300 commented May 29, 2019

Maybe we should test that this works fine even if a toString throws, since some of the implementations use a custom depth counter.

Simn pushed a commit to HaxeFoundation/hxcpp that referenced this pull request Jun 5, 2019
* Prevent infinite recustion on Array.toString (HaxeFoundation/haxe#8113)

* minor

* codestyle
@Simn Simn merged commit ed9b71e into HaxeFoundation:development Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[lua] Printing of recursive values causes stack overflow
6 participants