I don't know this part neither, just did what every other C developer would do: RTFM, fail to interpret TFM, RTFM again and think.
2017-11-08 22:56 GMT+01:00 Ben Coman <[email protected]>: > Thanks for your deep dive on this Nicolas. > I'm not familiar with that part of the system, and its interesting to read > about the interface constraints. > > cheers -ben > > > On Wed, Nov 8, 2017 at 11:40 PM, Nicolas Cellier < > [email protected]> wrote: > >> >> >> 2017-11-08 16:10 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@gmai >> l.com>: >> >>> >>> >>> 2017-11-08 15:35 GMT+01:00 Nicolas Cellier < >>> [email protected]>: >>> >>>> >>>> >>>> 2017-11-08 14:53 GMT+01:00 Nicolas Cellier < >>>> [email protected]>: >>>> >>>>> >>>>> >>>>> 2017-11-08 14:42 GMT+01:00 Nicolas Cellier < >>>>> [email protected]>: >>>>> >>>>>> >>>>>> 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! >>> >>> >> No total misunderstanding from my side... >> by setting the user data and destroy function, we are asking to >> auto-release the ft_face >> In which case, it's bad, because the ft_face is not ref-counted and we >> might still have another pointer on it from within Smalltalk >> >> The only way is thus to poll thru: >> >> cairo_font_face_get_reference_count (*cairo_font_face_t >> <https://www.cairographics.org/manual/cairo-cairo-font-face-t.html#cairo-font-face-t> >> *font_face*); >> >> If the count is 1, we can safely proceed with finalization. >> Else, there is still an internal reference somewhere in cairo and bang! >> >> I'd suggest to instrument the code with this function: >> >> CairoFontFace class>>countReferences: handle >> " >> unsigned int cairo_font_face_get_reference_count >> (cairo_font_face_t *font_face); >> " >> <primitive: #primitiveNativeCall module: #NativeBoostPlugin error: >> errorCode> >> ^ self nbCall: #( unsigned int cairo_font_face_get_reference_count >> (size_t handle)) >> >> >> CairoFontFace class>>reallyFinalizeResourceData: handle >> " >> void cairo_font_face_destroy >> (cairo_font_face_t *font_face); >> " >> <primitive: #primitiveNativeCall module: #NativeBoostPlugin error: >> errorCode> >> ^ self nbCall: #( void cairo_font_face_destroy (size_t handle)) >> >> CairoFontFace class>>finalizeResourceData: handle >> (self countReferences: handle) = 1 ifFalse: [self halt: 'Houston, >> we gonna have a pointer reference problem']. >> ^self reallyFinalizeResourceData: handle >> >> If suspicion is confirmed, then we'll have to install the ref_count >> polling mechanism... >> >> >> >>>> >>>>>> 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 <[email protected]>: >>>>>> >>>>>>> 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 < >>>>>>> [email protected]> 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 <[email protected]>: >>>>>>>> >>>>>>>>> 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 < >>>>>>>>> [email protected]> 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 >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >>> >> >
