[MacRuby] #1313: #to_json will cause a stack overflow if generating a collection with objective-c objects
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction ------------------------------------+--------------------------------------- Reduction: {{{ require 'json' [NSString.alloc.initWithString('')].to_json }}} It looks like the issue is caused because the JSON library does not match a type against an NSString, so it tries to generate JSON using the value of #to_s, which returns another NSString. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction ------------------------------------+--------------------------------------- Comment(by watson1978@…): 'json/pure' also crash with stack overflow. {{{ $ macruby -r json/pure -e "[NSString.alloc.initWithString('')].to_json" zsh: segmentation fault macruby -r json/pure -e "[NSString.alloc.initWithString('')].to_json" }}} -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:1> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction ------------------------------------+--------------------------------------- Comment(by mrada@…): It seems that all core types are not handled properly by the generate_json function. Other objective-c types return an NSString when #to_s is called. However, simply changing generate_json to call String() instead of #to_s, or even just handling NSString, will only fix crashing. The generator will still not generate the correct JSON for objective-c core types. This is a fairly major regression in the library and I think it should be a 0.11 blocker. {{{ framework 'Cocoa' [NSDictionary.alloc.initWithObjectsAndKeys(1,2,nil)].to_json }}} -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:2> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Changes (by lsansonetti@…): * keywords: reduction => reduction 0.11-blocker Comment: Agreed, adding 0.11-blocker keyword. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:3> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by watson1978@…): Does it have problem if NSString#to_s would return a RubyString? {{{ #!diff diff --git a/NSString.m b/NSString.m index 99f47db..4333021 100644 --- a/NSString.m +++ b/NSString.m @@ -64,7 +64,14 @@ nsstr_dup(id rcv, SEL sel) static id nsstr_to_s(id rcv, SEL sel) { - return rcv; + id rstr = (id)str_new_from_cfstring((CFStringRef)rcv); + if (OBJ_TAINTED(rcv)) { + OBJ_TAINT(rstr); + } + if (![rcv respondsToSelector:@selector(setString:)]) { + OBJ_FREEZE(rstr); + } + return rstr; } static id }}} -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:4> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by mrada@…): The problem with that solution is that it would only stop the crash. We would still be generating incorrect JSON for NSDictionary since NSDictionary#to_s will be called and it does not generate a valid JSON hash. Then, there is the additional problem that the Object#to_json method is not defined when loading the ext version of json, which means that code that depends on #to_json to be defined on all objects will break. I think the best solution to the problem is https://github.com/ferrous26/MacRuby/commit/bd65650315b, this is because of the way that the generator methods are mixed in the proper classes. However, the same fix cannot be applied to C version of json because of #1326. The other possible fix is to make a change to the method that mixes the generators in so that it mixes them in to the proper Foundation classes. Then there is still https://github.com/ferrous26/MacRuby/commit/8a0f79a5122c8, which adds proper detection, otherwise we are depending on the #to_json or the #to_json.to_s catch-all at the bottom which just adds needless overhead to the generation. Though I think this is something I can push upstream. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:5> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by watson1978@…): I don't think best solution to modified the JSON. I think that similar problem will happen in other RubyGems. Whenever problems occur, I think that modifing to the library is hard. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:6> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by mrada@…): I don't want to modify json either, but I don't think we can fix this problem without making at least one or two small changes. You have already made one change before that we have to revert, but is blocked by #1326. I would prefer the least conceptual change, which is what I linked to earlier in my MR fork; but the alternative solution I proposed would be contained to a single file where we modify a single method. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:7> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by mrada@…): Now that I thought about it some more, I think the second solution would be best. The difference in conceptual changes is about the same, but the second solution is more contained and is not blocked by #1326. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:8> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by mrada@…): Wait, what am I thinking, that's not right, we still have to deal with https://github.com/MacRuby/MacRuby/blob/master/ext/json/generator/generator.... Which is blocked by #1326, the only workaround is to name it differently and handle that special case when we override https://github.com/MacRuby/MacRuby/blob/master/ext/json/lib/json/common.rb#L... to handle NSString, NSArray, and NSDictionary. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:9> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: new Priority: critical | Milestone: Component: MacRuby | Keywords: reduction 0.11-blocker ------------------------------------+--------------------------------------- Comment(by mrada@…): A minimum fix is now applied in https://github.com/MacRuby/MacRuby/commit/3809805cfe11db21d96fca60b06f62f11e... Which should be sufficient for this issue to be closed. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:10> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: closed Priority: critical | Milestone: MacRuby 0.11 Component: MacRuby | Resolution: fixed Keywords: reduction 0.11-blocker | ------------------------------------+--------------------------------------- Changes (by lsansonetti@…): * status: new => closed * resolution: => fixed * milestone: => MacRuby 0.11 Comment: Thanks, this is good enough for 0.11. We can revisit a better fix for the next release :) -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:11> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: reopened Priority: critical | Milestone: MacRuby 0.11 Component: MacRuby | Resolution: Keywords: reduction 0.11-blocker | ------------------------------------+--------------------------------------- Changes (by mrada@…): * status: closed => reopened * resolution: fixed => Comment: Re-opening the ticket to track the performance loss when encoding foundation core types. Since the json generation is working, it is no longer a blocker, so I'll need someone to remove the tag for me... -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:12> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: reopened Priority: critical | Milestone: MacRuby 0.11 Component: MacRuby | Resolution: Keywords: | ------------------------------------+--------------------------------------- Changes (by watson1978@…): * keywords: reduction 0.11-blocker => Comment: delete a "0.11-blocker" keyword. -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:13> MacRuby <http://macruby.org/>
#1313: #to_json will cause a stack overflow if generating a collection with objective-c objects ------------------------------------+--------------------------------------- Reporter: mrada@… | Owner: lsansonetti@… Type: defect | Status: reopened Priority: critical | Milestone: MacRuby 0.11 Component: MacRuby | Resolution: Keywords: | ------------------------------------+--------------------------------------- Comment(by mrada@…): Some simple benchmarks at https://gist.github.com/1064281 -- Ticket URL: <http://www.macruby.org/trac/ticket/1313#comment:14> MacRuby <http://macruby.org/>
participants (1)
-
MacRuby