ExternalAddress uniqueness (was Re: Call for help for stability and rewrite of FreeType)
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.self@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
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 <btc@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.self@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
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.nice@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 <btc@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.self@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
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... 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.alios@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.nice@ 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 <btc@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.self@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
2017-11-08 14:42 GMT+01:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.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...
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.alios@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.nice@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 <btc@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.self@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
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@ gmail.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?
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.alios@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.nice@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 <btc@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.self@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
2017-11-08 15:35 GMT+01:00 Nicolas Cellier < nicolas.cellier.aka.nice@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.alios@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.nice@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 <btc@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.self@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
2017-11-08 16:10 GMT+01:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
2017-11-08 15:35 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@ gmail.com>:
2017-11-08 14:53 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@gmai l.com>:
2017-11-08 14:42 GMT+01:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.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!
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...> *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 <kilon.alios@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.nice@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 <btc@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.self@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
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 < nicolas.cellier.aka.nice@gmail.com> wrote:
2017-11-08 16:10 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@ gmail.com>:
2017-11-08 15:35 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@gmai l.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@gmail.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!
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...> *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 <kilon.alios@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.nice@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 <btc@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.self@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
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 <btc@openinworld.com>:
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 < nicolas.cellier.aka.nice@gmail.com> 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 < nicolas.cellier.aka.nice@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@gmail.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!
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...> *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 <kilon.alios@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.nice@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 <btc@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.self@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
participants (3)
-
Ben Coman -
Dimitris Chloupis -
Nicolas Cellier