David TurnerWhy is it impossible to free struct lock_files? I understand that they become part of a linked list, and that there's an atexit handler that goes over that list. But couldn't we just remove them from the linked list and then free them? Even if we couldn't free all lock_files, maybe we could free some? Anyway, just curious why that decision was made.
Junio C HamanoRe: Freeing struct lock_file?
David Turner <dturner@twopensource.com> writes:
> Why is it impossible to free struct lock_files? I understand that they > become part of a linked list, and that there's an atexit handler that > goes over that list. But couldn't we just remove them from the linked > list and then free them?
I suspect that the code is worried about getting a signal, while it is manipulating the linked list, and then cause the atexit handler to walk a list that is in a broken state.
David TurnerRe: Freeing struct lock_file?
On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:
> David Turner <dturner@twopensource.com> writes: > > > Why is it impossible to free struct lock_files? I understand that they > > become part of a linked list, and that there's an atexit handler that > > goes over that list. But couldn't we just remove them from the linked > > list and then free them? > > I suspect that the code is worried about getting a signal, while it > is manipulating the linked list, and then cause the atexit handler > to walk a list that is in a broken state.
This is technically possible, but practically unlikely: aligned pointer-sized writes are atomic on very nearly every processor, and that is all that is required to remove an item from a linked list safely in this case (though not, of course, in the general multi-threaded case).
But I can see why git wouldn't want to depend on that behavior. C11 has a way to do this safely, but AIUI, git doesn't want to move to C99 let alone C11. So I guess this will just have to remain the way it is.
Torsten BögershausenRe: Freeing struct lock_file?
On 2015-04-04 02.24, David Turner wrote:
> On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote: >> David Turner <dturner@twopensource.com> writes: >> >>> Why is it impossible to free struct lock_files? I understand that they >>> become part of a linked list, and that there's an atexit handler that >>> goes over that list. But couldn't we just remove them from the linked >>> list and then free them? >> >> I suspect that the code is worried about getting a signal, while it >> is manipulating the linked list, and then cause the atexit handler >> to walk a list that is in a broken state. > > This is technically possible, but practically unlikely: aligned > pointer-sized writes are atomic on very nearly every processor, and that > is all that is required to remove an item from a linked list safely in > this case (though not, of course, in the general multi-threaded case). > > But I can see why git wouldn't want to depend on that behavior. C11 has > a way to do this safely, but AIUI, git doesn't want to move to C99 let > alone C11. So I guess this will just have to remain the way it is. >
If you insist on using C11, may be.
But if there is an implementation that is #ifdef'ed and only enabled for "known to work processors" and a no-op for the others, why not ?
Do you have anything in special in mind ? A "git diff" may be a start for a patch series..
David TurnerRe: Freeing struct lock_file?
On Sat, 2015-04-04 at 09:16 +0200, Torsten Bögershausen wrote:
> On 2015-04-04 02.24, David Turner wrote: > > On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote: > >> David Turner <dturner@twopensource.com> writes: > >> > >>> Why is it impossible to free struct lock_files? I understand that they > >>> become part of a linked list, and that there's an atexit handler that > >>> goes over that list. But couldn't we just remove them from the linked > >>> list and then free them? > >> > >> I suspect that the code is worried about getting a signal, while it > >> is manipulating the linked list, and then cause the atexit handler > >> to walk a list that is in a broken state. > > > > This is technically possible, but practically unlikely: aligned > > pointer-sized writes are atomic on very nearly every processor, and that > > is all that is required to remove an item from a linked list safely in > > this case (though not, of course, in the general multi-threaded case). > > > > But I can see why git wouldn't want to depend on that behavior. C11 has > > a way to do this safely, but AIUI, git doesn't want to move to C99 let > > alone C11. So I guess this will just have to remain the way it is. > > > If you insist on using C11, may be. > > But if there is an implementation that is #ifdef'ed and only enabled for > "known to work processors" and a no-op for the others, why not ? > > Do you have anything in special in mind ? > A "git diff" may be a start for a patch series..
I haven't written any code for this yet. I wanted to understand the current code first.
My major worry is be that the code would be somewhat fragile as it depends on not just the processor, but also the C compiler's structure packing rules, which are implementation-dependent. In practice, major compilers' rules are safe, but it's annoying to have to depend on (especially since any bugs would be incredibly difficult to reproduce).
brian m. carlsonC99 (Was: Re: Freeing struct lock_file?)
On Fri, Apr 03, 2015 at 08:24:43PM -0400, David Turner wrote:
> But I can see why git wouldn't want to depend on that behavior. C11 has > a way to do this safely, but AIUI, git doesn't want to move to C99 let > alone C11. So I guess this will just have to remain the way it is.
I would really like to move to at least C99. However, MSVC bundles a C89 compiler and a C++ compiler, but Microsoft steadfastly refuses to bundle an actual C99 compiler, and Git doesn't even come close to compiling as C++. Newer versions of MSVC (2013 and newer) support enough of the features, though, that it might be possible.
So it isn't as much of a "we don't want to move to C99" as much as "we aren't yet willing to drop support for older versions of MSVC". I don't use Windows, so I'm happy to drop support for MSVC 2012 and older, but others may have different opinions.
-- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Junio C HamanoRe: C99
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> So it isn't as much of a "we don't want to move to C99" as much as "we > aren't yet willing to drop support for older versions of MSVC".
I do not particularly like that 'we' in that sentence, which would give a false impression to people that we all want to switch and MSVC is the only thing that is holding us back.
brian m. carlsonRe: C99
On Sat, Apr 04, 2015 at 01:06:43PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes: > > > So it isn't as much of a "we don't want to move to C99" as much as "we > > aren't yet willing to drop support for older versions of MSVC". > > I do not particularly like that 'we' in that sentence, which would > give a false impression to people that we all want to switch and > MSVC is the only thing that is holding us back.
Okay, I should have clarified my statement. I appreciate the correction.
Some people would like to move to C99, and so far the major objections I've heard have been that MSVC doesn't support it and that C99's benefits are unclear. I didn't meant to speak for others in that we should or should not, and there might be other objections I don't recall or haven't heard.
I seem to recall Peff wanting to use variadic macros at some point, although I can't recall specifically where. We also already use hacks to implement some of the features and hope that compilers will DTRT.
All the major compilers I'm aware of other than MSVC support C99, at least well enough to do the things we'd likely end up doing.
-- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
David TurnerRe: Freeing struct lock_file?
On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:
> David Turner <dturner@twopensource.com> writes: > > > Why is it impossible to free struct lock_files? I understand that they > > become part of a linked list, and that there's an atexit handler that > > goes over that list. But couldn't we just remove them from the linked > > list and then free them? > > I suspect that the code is worried about getting a signal, while it > is manipulating the linked list, and then cause the atexit handler > to walk a list that is in a broken state.
Actually, couldn't we just block signals with sigprocmask while manipulating the list?