search

groue / GRMustache.swift

Flexible Mustache templates for Swift
http://mustache.github.com/
MIT License
597 stars 155 forks source link

Using template inheritance throws malloc error #3

Closed Orion98MC closed 9 years ago

Orion98MC commented 9 years ago

Hi,

I tried to make a simple project with 1 file inheriting from a layout file and it crashes with this error:

malloc: * error for object 0xfffffffc: Invalid signature for pointer dequeued from free list * set a breakpoint in malloc_error_break to debug

It is worth to note that I then tried the very same 2 files with GRMustache and it works without error

For the record here is the code with GRMustache.swift:

          var error: NSError?
          if let template = Template(named: "link", bundle: webRootBundle, templateExtension: "html", encoding: NSUTF8StringEncoding, error: &error) {

            var person = Person(name: "Foo", age: 12)

            if let string = template.render(Box(person), error: &error) {
              NSLog("html: %@", string)
              // Crashes here!
            }
        }

Here is the one also from swift but using GRMustache bridged:

var error: NSError?
        var templateRepository = GRMustacheTemplateRepository(baseURL: webRootBundle.resourceURL!, templateExtension: "html", encoding: NSUTF8StringEncoding)

        if let template = templateRepository.templateNamed("link", error: &error) {

          var person = Person(name: "Foo", age: 12)

          if let string = template.renderObject(person, error: &error) {
            NSLog("html: %@", string)
            // Works
          }

The version I was using was "master" no tag.

Regards, Thierry

groue commented 9 years ago

Hello @Orion98MC,

I'm quite happy you try GRMustache.swift, and am sorry about this error.

I don't quite know yet what is happening. Here are two questions for you:

Orion98MC commented 9 years ago

Hi,

I'm afraid there is no stack trace on this one. just the error I pasted you. And It's stuck in in the main thread on the AppDelegate (which has no code at all) It looks like some garbage collection issue... But I have no other idea.

It's crashed on "debug" scheme.

While on the subject. Is GRMustache App Store compliant ? I'm much concerned about method swizzling.

Regards, Thierry

groue commented 9 years ago

I'm afraid there is no stack trace on this one. just the error I pasted you. And It's stuck in in the main thread on the AppDelegate (which has no code at all) It looks like some garbage collection issue... But I have no other idea.

All right. But that's not enough for me to work on the issue, assuming it belongs to GRMustache.swift.

While on the subject. Is GRMustache App Store compliant ? I'm much concerned about method swizzling

GRMustache is currently used by many apps in the App Store.

Anyway, method swizzling is not, as far as I know, a technique forbidden by Apple. And even though, GRMustache will swizzle methods if and only if you call +[GRMustache preventNSUndefinedKeyExceptionAttack], which you should not do in the release configuration since it is a debugging tool (documentation).

There is no reason to worry.

groue commented 9 years ago

...And GRMustache.swift does not swizzle anything at all.

Orion98MC commented 9 years ago

Here is a lldb bt result:

TestWebview(37789,0x7fff7e058300) malloc: *** error for object 0xfffffffc: Invalid signature for pointer dequeued from free list
*** set a breakpoint in malloc_error_break to debug
(lldb) bt
* thread #1: tid = 0x60b7cd, 0x00007fff93929286 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00007fff93929286 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff939de42f libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fff8fa10b53 libsystem_c.dylib`abort + 129
    frame #3: 0x00007fff9a958f3c libsystem_malloc.dylib`nanozone_error + 524
    frame #4: 0x00007fff9a9469a1 libsystem_malloc.dylib`_nano_malloc_check_clear + 183
    frame #5: 0x00007fff9a9468cc libsystem_malloc.dylib`nano_malloc + 35
    frame #6: 0x00007fff9a946877 libsystem_malloc.dylib`malloc_zone_malloc + 71
    frame #7: 0x00007fff92523e0e CoreFoundation`_CFRuntimeCreateInstance + 350
    frame #8: 0x00007fff95fdc8c4 CoreText`CTLineCreateWithAttributedString + 74
    frame #9: 0x00007fff9bfa4da1 HIToolbox`TCoreTextEngine::LayoutSingleLine(THIThemeTextInfo*, float) + 37
    frame #10: 0x00007fff9bfa4d3f HIToolbox`TCoreTextEngine::Layout(THIThemeTextInfo*, float, float, TextLayoutType) + 95
    frame #11: 0x00007fff9bfa4aa7 HIToolbox`TCoreTextEngine::GetThemeTextDimensions(double, THIThemeTextInfo*, double*, double*, double*) + 43
    frame #12: 0x00007fff9bfa3849 HIToolbox`DataEngine::GetTextDimensions(void const*, double, HIThemeTextInfo*, double*, double*, double*) + 305
    frame #13: 0x00007fff9bfa36e1 HIToolbox`HIThemeGetTextDimensions + 185
    frame #14: 0x00007fff9bfa31e5 HIToolbox`HIMenuBarView::MeasureMenuTitle(MenuData*, unsigned char, float*, int) + 465
    frame #15: 0x00007fff9bfa2f56 HIToolbox`HIMenuBarView::MeasureAppMenus() + 236
    frame #16: 0x00007fff9bfa2b17 HIToolbox`HIMenuBarView::EnsureBarLayout() + 485
    frame #17: 0x00007fff9bfa21e4 HIToolbox`HIMenuBarView::DrawOnce(CGRect, CGRect, bool, bool, CGContext*) + 532
    frame #18: 0x00007fff9bfa1c71 HIToolbox`HIMenuBarView::DrawSelf(short, __HIShape const*, CGContext*) + 559
    frame #19: 0x00007fff9bfa157d HIToolbox`HIView::DrawCacheOrSelf(short, __HIShape const*, CGContext*) + 343
    frame #20: 0x00007fff9bf918aa HIToolbox`HIView::EventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 2990
    frame #21: 0x00007fff9bf85b6c HIToolbox`DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1260
    frame #22: 0x00007fff9bf84fae HIToolbox`SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 386
    frame #23: 0x00007fff9bf84e22 HIToolbox`SendEventToEventTargetWithOptions + 43
    frame #24: 0x00007fff9bfa13b9 HIToolbox`HIView::SendDraw(short, OpaqueGrafPtr*, __HIShape const*, CGContext*) + 337
    frame #25: 0x00007fff9bfa0d42 HIToolbox`HIView::RecursiveDrawComposited(__HIShape const*, __HIShape const*, unsigned int, HIView*, CGContext*, unsigned char, double) + 710
    frame #26: 0x00007fff9bfa0fbe HIToolbox`HIView::RecursiveDrawComposited(__HIShape const*, __HIShape const*, unsigned int, HIView*, CGContext*, unsigned char, double) + 1346
    frame #27: 0x00007fff9bf9fcdb HIToolbox`HIView::DrawComposited(short, OpaqueGrafPtr*, __HIShape const*, unsigned int, HIView*, CGContext*) + 965
    frame #28: 0x00007fff9bf9f904 HIToolbox`HIView::Render(unsigned int, CGContext*) + 54
    frame #29: 0x00007fff9bf9f028 HIToolbox`WindowData::PrepareForVisibility() + 168
    frame #30: 0x00007fff9bf9ded8 HIToolbox`_ShowHideWindows + 393
    frame #31: 0x00007fff9bf9dd49 HIToolbox`ShowHide + 34
    frame #32: 0x00007fff9bf8fced HIToolbox`MBWindows::CreateWindow(CGRect, unsigned int) + 377
    frame #33: 0x00007fff9bf8fa56 HIToolbox`MBWindows::GetWindowIDOnDisplay(unsigned int, unsigned char) + 174
    frame #34: 0x00007fff9bf8f84e HIToolbox`MenuBarInstance::ForEachWindowDo(void (unsigned int, unsigned int) block_pointer) + 162
    frame #35: 0x00007fff9bf8f5ad HIToolbox`MenuBarInstance::UpdateWindowBoundsAndResolution() + 155
    frame #36: 0x00007fff9bf8f2c7 HIToolbox`MenuBarInstance::Show(MenuBarAnimationStyle, unsigned char, unsigned char, unsigned char) + 229
    frame #37: 0x00007fff9bfc0e85 HIToolbox`SetMenuBarObscured + 232
    frame #38: 0x00007fff9bfc0ade HIToolbox`HIApplication::HandleActivated(OpaqueEventRef*, unsigned char, OpaqueWindowPtr*) + 184
    frame #39: 0x00007fff9bfbd700 HIToolbox`HIApplication::EventObserver(unsigned int, OpaqueEventRef*, void*) + 238
    frame #40: 0x00007fff9bf8530c HIToolbox`_NotifyEventLoopObservers + 155
    frame #41: 0x00007fff9bfbd282 HIToolbox`AcquireEventFromQueue + 821
    frame #42: 0x00007fff9bfaf225 HIToolbox`ReceiveNextEventCommon + 234
    frame #43: 0x00007fff9bfaf12b HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71
    frame #44: 0x00007fff964c29bb AppKit`_DPSNextEvent + 978
    frame #45: 0x00007fff964c1f68 AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346
    frame #46: 0x00007fff964b7bf3 AppKit`-[NSApplication run] + 594
    frame #47: 0x00007fff96434354 AppKit`NSApplicationMain + 1832
  * frame #48: 0x000000010000cced TestWebview`main + 109 at AppDelegate.swift:12
    frame #49: 0x00007fff8db1a5c9 libdyld.dylib`start + 1
    frame #50: 0x00007fff8db1a5c9 libdyld.dylib`start + 1
Orion98MC commented 9 years ago

GRMustache is currently used by many apps in the App Store.

Ok thanks for the info.

I'd like to use the swift version but since it crashes I could backup to the Objc version for now.

groue commented 9 years ago

In the blob you have just pasted, I don't see anything related to the NSLog line which is supposed to crash?

Orion98MC commented 9 years ago

I could send you a tar of the test project so you can figure it out... Other than that I'm afraid I cannot help you dig this issue ... my email is on my github page...

groue commented 9 years ago

I'd like to use the swift version but since it crashes I could backup to the Objc version for now.

Yes, GRMustache.swift is quite young. It takes time (and bugs) for a library to become very robust :-)

Orion98MC commented 9 years ago

In the blob you have just pasted, I don't see anything related to the NSLog line which is supposed to crash?

Yep, that's why I think it's a gc kind of error

groue commented 9 years ago

Yep, that's why I think it's a gc kind of error

GC as in garbage collector? Is there a gc in swift? I don't follow you. What do you mean?

Orion98MC commented 9 years ago

Yes, GRMustache.swift is quite young. It takes time (and bugs) for a library to become very robust :-)

I'm more than thankful to people who make open source softwares. So I know what you are talking about :)

Orion98MC commented 9 years ago

GC as in garbage collector? Is there a gc in swift? I don't follow you. What do you mean?

well, swift run on top of the objc runtime which is garbage collected (autorelease pools and stuff)

groue commented 9 years ago

OK. That "GC" is called ARC (Automatic Reference Counting), actually.

I have to leave now. But we'll look at your issue later. Bon week-end, Thierry :-)

Orion98MC commented 9 years ago

Ok bon week end aussi.

Orion98MC commented 9 years ago

Just to add up, after enabling most of the debug flags in the debug scheme I can get this back trace which has more info:


GuardMalloc[TestWebview-40528]: Allocations will be placed on 16 byte boundaries.
GuardMalloc[TestWebview-40528]:  - Some buffer overruns may not be noticed.
GuardMalloc[TestWebview-40528]:  - Applications using vector instructions (e.g., SSE) should work.
GuardMalloc[TestWebview-40528]: version 104
(lldb) bt
* thread #1: tid = 0x61aaf7, 0x00000001003f99ba libswiftCore.dylib`_swift_release_(swift::HeapObject*) + 10, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x165afcf18)
    frame #0: 0x00000001003f99ba libswiftCore.dylib`_swift_release_(swift::HeapObject*) + 10
    frame #1: 0x000000010007d57b Mustache`Mustache.RenderingEngine.__deallocating_deinit [inlined] Mustache.RenderingEngine.deinit + 18 at <unknown>:0
    frame #2: 0x000000010007d569 Mustache`Mustache.RenderingEngine.__deallocating_deinit(self=(null) at rbx) + 9 at RenderingEngine.swift:0
    frame #3: 0x000000010009b88c Mustache`Mustache.Template.render (box=<unavailable>, error=<unavailable>, self=<unavailable>)(Mustache.MustacheBox, error : Swift.AutoreleasingUnsafeMutablePointer<Swift.Optional<ObjectiveC.NSError>>) -> Swift.Optional<Swift.String> + 3244 at Template.swift:0
  * frame #4: 0x0000000100007341 TestWebview`TestWebview.ViewController.viewDidAppear (self=0x00000001134fef90)() -> () + 4833 at ViewController.swift:43
    frame #5: 0x0000000100007a22 TestWebview`@objc TestWebview.ViewController.viewDidAppear (TestWebview.ViewController)() -> () + 34 at ViewController.swift:0
    frame #6: 0x00007fff9662508e AppKit`-[NSViewController _sendViewDidAppear] + 160
    frame #7: 0x00007fff9657adbe AppKit`-[NSView(NSInternal) _windowDidOrderOnScreen] + 67
    frame #8: 0x00007fff9657aea0 AppKit`-[NSView(NSInternal) _windowDidOrderOnScreen] + 293
    frame #9: 0x00007fff96568d36 AppKit`-[NSWindow _reallyDoOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 3915
    frame #10: 0x00007fff96567897 AppKit`-[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 829
    frame #11: 0x00007fff965674eb AppKit`-[NSWindow orderWindow:relativeTo:] + 159
    frame #12: 0x00007fff9655eab2 AppKit`-[NSWindow makeKeyAndOrderFront:] + 47
    frame #13: 0x00007fff96641f27 AppKit`__33-[NSWindowController showWindow:]_block_invoke + 367
    frame #14: 0x00007fff93ca35f6 QuickLookUI`-[QLSeamlessDocumentOpener showWindow:contentFrame:withBlock:] + 105
    frame #15: 0x00007fff965f7e50 AppKit`-[NSWindowController showWindow:] + 434
    frame #16: 0x00007fff96434064 AppKit`NSApplicationMain + 1080
    frame #17: 0x000000010000dcdd TestWebview`main + 109 at AppDelegate.swift:12
    frame #18: 0x00007fff8db1a5c9 libdyld.dylib`start + 1
groue commented 9 years ago

OK Thierry I could reproduce the bug from your sample project. The crash disappears when I remove the template inheritance (by removing the {{< layout }} and {{/ layout }} tags in the file link.html). I don't have any idea yet. But the evidence is there, ready to be investigated!

groue commented 9 years ago

OK. More information : the crash only happens with the framework built by Carthage included in your sample project.

It does not happen when I embed the Mustache.xcodeproj project in your project, and set its MustacheOSX target as a dependency of your TestWebView target (replacing the link to the Carthage framework).

What if Carthage has built a rotten framework? Is it really a GRMustache.swift issue? I lack experience on Catharge.

groue commented 9 years ago

All right, more information again: the bug appears again if the app runs in Release Configuration. Now I guess Xcode builds the same GRMustache framework as Carthage, and the conditions for the crash are back.

Orion98MC commented 9 years ago

sounds like a "Release" compiler optimisation messes the whole thing...

groue commented 9 years ago

Yes. Issue #1 also happened in Release configuration. Young language, many bugs! Yet #1 involved Swift generics. This time it looks different.

My test suite also crashes when it runs inheritance tests in Release configuration. It doesn't look exactly like your crash, though. Investigating...

Orion98MC commented 9 years ago

Good luck, if I can help don't hesitate.

Orion98MC commented 9 years ago

Between, I'm updating to Xcode 6.3.1 at the moment...

groue commented 9 years ago

Hi Thierry @Orion98MC ! I believe the issue is fixed now. Just pull the master branch, have Carthage rebuild the framework, and please tell me that everything is fine :-)

Orion98MC commented 9 years ago

Indeed it does work now! :)

What was the problem?

groue commented 9 years ago

Cool, glad it worked :-)

The problem was lying at these lines: https://github.com/groue/GRMustache.swift/blob/bedaf7fa6010c23a13e01612a5feaa718e5ed2a2/Mustache/Rendering/RenderingEngine.swift#L50-L54

func visit(inheritedPartialNode: InheritedPartialNode) -> TemplateASTVisitResult {
    let originalContext = context
    context = context.extendedContext(inheritedPartialNode: inheritedPartialNode)
    let result = visit(inheritedPartialNode.partialNode)
    context = originalContext
    return result
}

The visit(InheritedPartialNode) function above is recursive because there may be several levels of inheritance . The originalContext local variable is used as way to store on the stack the current rendering context (let originalContext = context), before resetting it before the function returns (context = originalContext).

This works as expected in Debug configuration, but corrupts memory in Release configuration, because of a Swift compiler bug.

The workaround involved avoiding this pattern. After refactoring, there is no longer any context ivar that stores the current rendering context. Instead, the context is now passed as function argument all along the rendering: https://github.com/groue/GRMustache.swift/blob/adc3fba0331a318d6e94744b3c2fba9c92b18865/Mustache/Rendering/RenderingEngine.swift#L64

That was it!

Orion98MC commented 9 years ago

Well, great finding. Too bad we all have to suffer from swift newness... but hey ... It's new and awesome :) Anyways, thanks for this fix, I will include Mustache.swift in my project! Keep up.

groue commented 9 years ago

Well, I'll remember that all my Swift code must be checked regularly in Release configuration... The "fixes" may require refactoring.

BTW, with a little more comments, the new code may be more readable :-) https://github.com/groue/GRMustache.swift/blob/b3d7941ebb8ca50a7babc3e08ece8372a52010e1/Mustache/Rendering/RenderingEngine.swift#L120-L125

Happy Mustache, Thierry!