2017-11-08 15:35 GMT+01:00 Nicolas Cellier < nicolas.cellier.aka.n...@gmail.com>:
> > > 2017-11-08 14:53 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@ > gmail.com>: > >> >> >> 2017-11-08 14:42 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@gmai >> l.com>: >> >>> >>> Ben, >>> >>> This is my fresh crash.dmp >>> it sounds very related to your analysis!!! >>> >>> In fact we are not freeing by ourselves, but telling libgit2 to do it... >>> >>> >> Oh worse than that, it sounds like git implemented its own mechanism of >> counted pointers... >> So we don't tell anything, he guesses by himself. >> I would search for places where we #gcallocate: or manually #free a >> pointer on a structure passed back by git... >> >> > and of course, it's not gcallocate: because this was a very old wheel... > It's rather somewhere in UFFI equivalent > FFIExternalResourceExecutor <- FFIExternalResourceManager <- > LGitExternalStructure autoRelease > > Among the senders, we see (tiens, tiens...): > LGitTree>>... > > So this is where I would search the origin of my own crash dump... > > But also (tiens, tiens...): > CairoFontFace>>initializeWithFreetypeFace: > > What if FreeType plugin was not the problem per se, but its usage in cairo > was? > > cairo_font_face_destroy () > void cairo_font_face_destroy (cairo_font_face_t > *font_face); > Decreases the reference count on font_face by one. If the result is zero, > *then > font_face and all associated resources are freed*. See > cairo_font_face_reference(). > font_face : > a cairo_font_face_t > > Since we pass a pointer to the free type font at creation: > > fromFreetypeFace: aFace > | handle cairoFace | > handle := aFace handle pointerAt: 1. > cairoFace := self primFtFace: handle loadFlags: ( LoadNoHinting | > LoadTargetLCD | LoadNoAutohint | LoadNoBitmap). > ^ cairoFace initializeWithFreetypeFace: aFace > > Isn't it possible that we somehow double free the free type font too? > > Hmm not the exact catch but it could well be related https://www.cairographics.org/manual/cairo-FreeType-Fonts.html tells how to couple lifetime of the 2 data structures. I see that CairoFontFace retains a pointer on the FT_Face thru a dedicated ivar, so at least, we don't free the FT_Face twice, and we don't free it until we free the cairo_ft_face When finalizatoin occurs, I'm not sure that the finalization order is guaranteed but that does not matter. What matters is that the cairo_ft_face could still be referenced internally by cairo. So what can happen is that: 1) we don't reference anymore the CairoFontFace from within Smalltalk 2) finalization happens we call cairo_font_face_destroy () 3) there is no more pointer on the FTFace from within Smalltalk (because we just reclaimed the CairoFontFace pointing on it) 4) finalization happens and we call FT_Done_Face() BUT: cairo was still using the cairo_font_face internally, (the reference count did not reach zero) and is now pointing on freed memory due to FT_Done_Face()... We should have tested the status before invoking FT_Done(): status = cairo_font_face_set_user_data (font_face, &key, ft_face, (cairo_destroy_func_t) FT_Done_Face); That means that we would have to performa that status test in the finalization, and if not ready, keep a reference to both cairo_font_face handle ft_face handle But then there is no other mean than storing those reference in a safe place and regularly poll for readiness If my understanding is correct, this is absolutely garbage collector unfriendly! > >>> Stack backtrace: >>> [7791E43E] RtlInitializeGenericTable + 0x196 in ntdll.dll >>> [7791E0A3] RtlGetCompressionWorkSpaceSize + 0x7e in ntdll.dll >>> [751F98CD] free + 0x39 in msvcrt.dll >>> [6CD60D43] git_tree_cache_write + 0x2ac in libgit2.dll >>> [6CD62073] git_tree__free + 0x53 in libgit2.dll >>> [6CD1A563] git_object__free + 0x52 in libgit2.dll >>> [6CCD0D78] git_cached_obj_decref + 0x4c in libgit2.dll >>> [6CD1A7D9] git_object_free + 0x17 in libgit2.dll >>> [6CD1B0D3] git_tree_free + 0x11 in libgit2.dll >>> [6CD0BE4F] git_iterator_for_nothing + 0x8aa in libgit2.dll >>> [6CD0C053] git_iterator_for_nothing + 0xaae in libgit2.dll >>> [6CCEADEF] git_diff_file_content__clear + 0x31d in libgit2.dll >>> [6CCECC3F] git_diff__oid_for_entry + 0xc29 in libgit2.dll >>> [6CCED2B2] git_diff__oid_for_entry + 0x129c in libgit2.dll >>> [6CCED495] git_diff__from_iterators + 0x1db in libgit2.dll >>> [6CCED6DE] git_diff_tree_to_tree + 0x1e3 in libgit2.dll >>> [004DE7C8] ??? + 0xde7c8 in Pharo.exe >>> [0044FE08] ??? + 0x4fe08 in Pharo.exe >>> [004516A7] ??? + 0x516a7 in Pharo.exe >>> [00446051] ??? + 0x46051 in Pharo.exe >>> [0049936E] ??? + 0x9936e in Pharo.exe >>> >>> >>> Smalltalk stack dump: >>> 0xafa86c I LGitDiff>diff_tree_to_tree:repo:old_tree:new_tree:opts: >>> 0xe585410: a(n) LGitDiff >>> 0xafa8a4 M [] in LGitDiff>diffTree:toTree:options: 0xe585410: a(n) >>> LGitDiff >>> 0xafa8bc M LGitDiff(LGitExternalObject)>withReturnHandlerDo: >>> 0xe585410: a(n) LGitDiff >>> 0xafc678 I LGitDiff>diffTree:toTree:options: 0xe585410: a(n) LGitDiff >>> 0xafc6a4 I LGitDiff>diffTree:toTree: 0xe585410: a(n) LGitDiff >>> 0xafc6d0 I LGitTree>diffTo: 0xe583e00: a(n) LGitTree >>> 0xafc6fc M [] in IceLibgitLocalRepository>changedFilesBetween:and: >>> 0x1055afc0: a(n) IceLibgitLocalRepository >>> 0xafc720 M [] in IceLibgitLocalRepository>withRepoDo: 0x1055afc0: >>> a(n) IceLibgitLocalRepository >>> 0xafc73c M [] in LGitGlobal class>runSequence: 0xfb96188: a(n) >>> LGitGlobal class >>> 0xafc760 M [] in LGitActionSequence(DynamicVariable)>value:during: >>> 0x102109f8: a(n) LGitActionSequence >>> 0xafc780 M BlockClosure>ensure: 0xe582890: a(n) BlockClosure >>> 0xafc7ac I LGitActionSequence(DynamicVariable)>value:during: >>> 0x102109f8: a(n) LGitActionSequence >>> 0xafc7cc M LGitActionSequence class(DynamicVariable >>> class)>value:during: 0xfbb81e0: a(n) LGitActionSequence class >>> 0xafc7f4 I LGitGlobal class>runSequence: 0xfb96188: a(n) LGitGlobal >>> class >>> 0xafc818 I IceLibgitLocalRepository>withRepoDo: 0x1055afc0: a(n) >>> IceLibgitLocalRepository >>> 0xafc840 I IceLibgitLocalRepository>changedFilesBetween:and: >>> 0x1055afc0: a(n) IceLibgitLocalRepository >>> 0xafc874 I IceCommitInfo>changedPackagesToCommitInfo: 0x113b80e0: >>> a(n) IceCommitInfo >>> 0xafc898 I IceCommitInfo>changedPackagesTo: 0x113b80e0: a(n) >>> IceCommitInfo >>> 0xafc8c0 I IceDiff>initialElements 0xe4c48f8: a(n) IceDiff >>> 0xaf9664 I IceDiff(IceAbstractDiff)>elements 0xe4c48f8: a(n) IceDiff >>> 0xaf9684 I IceDiffChangeTreeBuilder>elements 0xe4b9c80: a(n) >>> IceDiffChangeTreeBuilder >>> 0xaf969c M [] in IceDiffChangeTreeBuilder>buildOn: 0xe4b9c80: a(n) >>> IceDiffChangeTreeBuilder >>> >>> Dimitris: >>> >>> I won't argument, I've learnt C in 1987, so it gave me enough time to >>> learn my own limits. >>> Working with pointers is like carrying a gun without engaging the safety >>> catch. >>> I came to think that shooting own foot was a feature ;) >>> >>> 2017-11-06 11:04 GMT+01:00 Dimitris Chloupis <kilon.al...@gmail.com>: >>> >>>> Its the usual case of not being able to have your cake and eat it too. >>>> >>>> If you want top performance you have to manage memory yourself plus the >>>> abilitiy to access thousands of C libraries is not such a bad excuse for a >>>> compromise. The FFI is not a problem is a solution to many problems and >>>> people using it its not as if Smalltalk offers them any alternative choice. >>>> >>>> Not to forget that Slang itself relies heavily on C, which is only the >>>> core of the VM and the very core of the implementation. >>>> >>>> Understanding how to work with pointers in C is pretty much >>>> understanding how to works with Objects in Smalltalk. Both are nuclear >>>> weapons that those two languages are build around. If ones does not >>>> understand their usage he will shoot his foot in the end. >>>> >>>> The important thing to remember is that C's goal is not the same as of >>>> Smalltalk. Its not there to hold your hand and make coding easy for you. C >>>> is there to offer low level access combined with top performance. It may >>>> have started as a general purpose language decades ago when coding in >>>> Assembly was still a pleasant experience. Nowdays C has completely replaced >>>> Assembly as the top performance language for low level coding. >>>> >>>> C may appear as a problematic language to a Smalltalker but only >>>> because he sees it from the Smalltalk point of view. The harsh reality of >>>> the world is that as much as one may want to shoehorn it , not everything >>>> can be elegantly mapped to a object. Smalltalk may be OO to the bone , but >>>> the world we live in, cannot afford such simple structures to accomodate of >>>> varied immense complexity. >>>> >>>> On the subject of pointers, the general rule of thumb is to keep things >>>> as simple as possible and avoide trying to do weird "magic" with them. >>>> There is a ton of things that C does under the hood to generate highly >>>> optimised machine code that can fry the brain , as the usual case with low >>>> level coding, so keeping it simple is the way to go. >>>> >>>> Oh and dont try to shoehorn the Live coding enviroment in debugging C >>>> code, as much as one may want to brag of Smalltalk's elegant debugger, C >>>> development tools are light years ahead in dealing with C problems. >>>> >>>> May advice to people is that if you do it via FFI first, you do it >>>> wrong. >>>> >>>> Do it always first with C with a powerful C IDE like Visual Studio, >>>> make sure your code works there and then use the UFFI. Will make life >>>> thousand times easier. I learned that the hard way when I was playing >>>> around with Pharo and shared memory. >>>> >>>> So yes having a FFI that does not help you avoid coding in C first, is >>>> a big plus, not a minus. Sometimes it makes sense to live outside the >>>> image, this is an excellent case to prove why that is a great idea. . >>>> >>>> On Mon, Nov 6, 2017 at 11:10 AM Nicolas Cellier < >>>> nicolas.cellier.aka.n...@gmail.com> wrote: >>>> >>>>> Hi Ben, >>>>> It's a super bad idea to copy an ExternalAddress. >>>>> It's common knowledge in C++ copy operator & copy constructors... >>>>> >>>>> But it's not obvious to me that you'll have double freeing (unless you >>>>> explicitely free the pointer by yourself). >>>>> If you use gcallocate: then only the original is registered for >>>>> magical auto-deallocation at garbage collection... >>>>> >>>>> What you will have is more somthing like dangling pointer: continue to >>>>> use pointer xa2->a1 when a1 was already freed. >>>>> >>>>> FFI is great, it introduces the problem of C in Smalltalk, augmented >>>>> with the problems of wrapping C in Smalltalk. >>>>> >>>>> >>>>> 2017-11-06 4:23 GMT+01:00 Ben Coman <b...@openinworld.com>: >>>>> >>>>>> My current employment work hours and roster have severely curtailed >>>>>> the time I have hacking Pharo, so I've not dug enough to be sure of my >>>>>> observations a few months ago, and this is from memory, but I was >>>>>> starting >>>>>> to develop a suspicion about the uniqueness of ExternalAddress(s). >>>>>> >>>>>> A while ago, in order to fix some stability issues on Windows, a >>>>>> guard was added somewhere that slowed down some operations. Looking into >>>>>> this and experimenting with removing the guard I seem to remember VM >>>>>> crashes due to a double-free() of an address, due to there being two >>>>>> ExternalAddresses holding the same external address. >>>>>> >>>>>> My intuition is that that somewhere an ExternalAddress(a1) pointing >>>>>> at a particular external resource address "xa1" was being copied, so we >>>>>> end >>>>>> up with ExternalAddress(a2) also pointing at "xa1", with and object b1 >>>>>> holding a1 and object b2 holding a2. During finalization of b1, >>>>>> ExternalAddress a1 free()d xa1, and a1 was flagged to avoid >>>>>> double-free()ing. But that didn't help when b2 was finalized, since a2 >>>>>> had >>>>>> no indication that xa1 had been free()d. >>>>>> >>>>>> That is... >>>>>> b1-->a1-->xa1 >>>>>> b2 := b1 copy. >>>>>> b2-->a2-->xa1 >>>>>> b1 finalize a1 --> free(xa1) >>>>>> b2 finalize a2 --> free(xa1) --> General Protection Fault >>>>>> >>>>>> It was hard to follow this through and I didn't succeed in tracking >>>>>> down where such a copy might have been made, but the idea simmering in my >>>>>> mind since then is to propose that... >>>>>> >>>>>> ExternalAddresses be unique in the image and behave like Symbols, >>>>>> such that trying to copy one returns the identical object. >>>>>> >>>>>> The idea being that when b2 is finalized, a1 would notice that xa1 >>>>>> had already been free()d and raise a Smalltalk exception rather than a >>>>>> general protection fault. >>>>>> b1-->a1-->xa1 >>>>>> b2 := b1 copy. >>>>>> b2-->a1-->xa1 >>>>>> ^^ >>>>>> b1 finalize a1 --> free(xa1) >>>>>> b2 finalize a1 --> Smalltalk exception >>>>>> >>>>>> >>>>>> I write now in response to Stef since I vaguely remember it being >>>>>> Freetype related. But I also remember the issue being FFI related and >>>>>> Freetype is a plugin not FFI. So I'm not sure my memory is clear and >>>>>> perhaps I have the "wrong end of the stick" but anyway, rather than hold >>>>>> back longer because of that, perhaps this can stimulate some discussion >>>>>> and >>>>>> at least I learn something to clarify my understanding here. >>>>>> >>>>>> cheers -ben >>>>>> >>>>>> >>>>>> On Sat, Oct 28, 2017 at 4:48 PM, Stephane Ducasse < >>>>>> stepharo.s...@gmail.com> wrote: >>>>>> > >>>>>> > Hi all >>>>>> > >>>>>> > I'm and I guess many of you are fedup about the instability that the >>>>>> > FreeType plugin produces. >>>>>> > >>>>>> > So we need help because clement and esteban are fully booked. >>>>>> > >>>>>> > We have three options: >>>>>> > >>>>>> > - drop Freetype alltogether >>>>>> > - rewrite the plugin >>>>>> > - create a binding using raffaillac sketch >>>>>> > >>>>>> > Now we need help. Who is willing to help us? >>>>>> > Should we try to set up a bounty? >>>>>> > >>>>>> > Stef >>>>>> >>>>>> >>>>>> >>>>> >>> >> >
