{"thread":{"id":"38995","subject":"Freeing struct lock_file?","startedAt":"2015-04-03T21:45:15Z","lastAt":"2015-04-07T01:12:43Z","messageCount":9,"participants":["David Turner","Junio C Hamano","Torsten Bögershausen","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"258949","messageId":"1428097515.31333.2.camel@ubuntu","threadId":"38995","inReplyTo":null,"subject":"Freeing struct lock_file?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-04-03T21:45:15Z","receivedAt":"2015-04-03T21:45:15Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Why is it impossible to free struct lock_files? I understand that they\nbecome part of a linked list, and that there's an atexit handler that\ngoes over that list. But couldn't we just remove them from the linked\nlist and then free them? Even if we couldn't free all lock_files, maybe\nwe could free some? Anyway, just curious why that decision was made.\n"},{"id":"258954","messageId":"xmqqfv8gg9bp.fsf@gitster.dls.corp.google.com","threadId":"38995","inReplyTo":"1428097515.31333.2.camel@ubuntu","subject":"Re: Freeing struct lock_file?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-03T22:01:46Z","receivedAt":"2015-04-03T22:01:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> Why is it impossible to free struct lock_files? I understand that they\n> become part of a linked list, and that there's an atexit handler that\n> goes over that list. But couldn't we just remove them from the linked\n> list and then free them? \n\nI suspect that the code is worried about getting a signal, while it\nis manipulating the linked list, and then cause the atexit handler\nto walk a list that is in a broken state.\n"},{"id":"258967","messageId":"1428107083.31333.11.camel@ubuntu","threadId":"38995","inReplyTo":"xmqqfv8gg9bp.fsf@gitster.dls.corp.google.com","subject":"Re: Freeing struct lock_file?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-04-04T00:24:43Z","receivedAt":"2015-04-04T00:24:43Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > Why is it impossible to free struct lock_files? I understand that they\n> > become part of a linked list, and that there's an atexit handler that\n> > goes over that list. But couldn't we just remove them from the linked\n> > list and then free them? \n> \n> I suspect that the code is worried about getting a signal, while it\n> is manipulating the linked list, and then cause the atexit handler\n> to walk a list that is in a broken state.\n\nThis is technically possible, but practically unlikely: aligned\npointer-sized writes are atomic on very nearly every processor, and that\nis all that is required to remove an item from a linked list safely in\nthis case (though not, of course, in the general multi-threaded case).\n\nBut I can see why git wouldn't want to depend on that behavior. C11 has\na way to do this safely, but AIUI, git doesn't want to move to C99 let\nalone C11. So I guess this will just have to remain the way it is.\n"},{"id":"258982","messageId":"551F8FC3.8010104@web.de","threadId":"38995","inReplyTo":"1428107083.31333.11.camel@ubuntu","subject":"Re: Freeing struct lock_file?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-04-04T07:16:19Z","receivedAt":"2015-04-04T07:16:19Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-04-04 02.24, David Turner wrote:\n> On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:\n>> David Turner writes:\n>>\n>>> Why is it impossible to free struct lock_files? I understand that they\n>>> become part of a linked list, and that there's an atexit handler that\n>>> goes over that list. But couldn't we just remove them from the linked\n>>> list and then free them? \n>>\n>> I suspect that the code is worried about getting a signal, while it\n>> is manipulating the linked list, and then cause the atexit handler\n>> to walk a list that is in a broken state.\n> \n> This is technically possible, but practically unlikely: aligned\n> pointer-sized writes are atomic on very nearly every processor, and that\n> is all that is required to remove an item from a linked list safely in\n> this case (though not, of course, in the general multi-threaded case).\n> \n> But I can see why git wouldn't want to depend on that behavior. C11 has\n> a way to do this safely, but AIUI, git doesn't want to move to C99 let\n> alone C11. So I guess this will just have to remain the way it is.\n> \nIf you insist on using C11, may be.\n\nBut if there is an implementation that is #ifdef'ed and only enabled for\n\"known to work processors\" and a no-op for the others, why not ?\n\nDo you have anything in special in mind ?\nA \"git diff\" may be a start for a patch series..\n"},{"id":"258994","messageId":"20150404190400.GB10125@vauxhall.crustytoothpaste.net","threadId":"38995","inReplyTo":"1428107083.31333.11.camel@ubuntu","subject":"C99 (Was: Re: Freeing struct lock_file?)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-04-04T19:04:00Z","receivedAt":"2015-04-04T19:04:00Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Apr 03, 2015 at 08:24:43PM -0400, David Turner wrote:\n> But I can see why git wouldn't want to depend on that behavior. C11 has\n> a way to do this safely, but AIUI, git doesn't want to move to C99 let\n> alone C11. So I guess this will just have to remain the way it is.\n\nI would really like to move to at least C99. However, MSVC bundles a\nC89 compiler and a C++ compiler, but Microsoft steadfastly refuses to\nbundle an actual C99 compiler, and Git doesn't even come close to\ncompiling as C++. Newer versions of MSVC (2013 and newer) support\nenough of the features, though, that it might be possible.\n\nSo it isn't as much of a \"we don't want to move to C99\" as much as \"we\naren't yet willing to drop support for older versions of MSVC\". I don't\nuse Windows, so I'm happy to drop support for MSVC 2012 and older, but\nothers may have different opinions.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"259000","messageId":"xmqqtwwvejzg.fsf@gitster.dls.corp.google.com","threadId":"38995","inReplyTo":"20150404190400.GB10125@vauxhall.crustytoothpaste.net","subject":"Re: C99","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-04T20:06:43Z","receivedAt":"2015-04-04T20:06:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> So it isn't as much of a \"we don't want to move to C99\" as much as \"we\n> aren't yet willing to drop support for older versions of MSVC\".\n\nI do not particularly like that 'we' in that sentence, which would\ngive a false impression to people that we all want to switch and\nMSVC is the only thing that is holding us back.\n"},{"id":"259001","messageId":"20150404203612.GD10125@vauxhall.crustytoothpaste.net","threadId":"38995","inReplyTo":"xmqqtwwvejzg.fsf@gitster.dls.corp.google.com","subject":"Re: C99","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-04-04T20:36:13Z","receivedAt":"2015-04-04T20:36:13Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Apr 04, 2015 at 01:06:43PM -0700, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > So it isn't as much of a \"we don't want to move to C99\" as much as \"we\n> > aren't yet willing to drop support for older versions of MSVC\".\n> \n> I do not particularly like that 'we' in that sentence, which would\n> give a false impression to people that we all want to switch and\n> MSVC is the only thing that is holding us back.\n\nOkay, I should have clarified my statement. I appreciate the\ncorrection.\n\nSome people would like to move to C99, and so far the major objections\nI've heard have been that MSVC doesn't support it and that C99's\nbenefits are unclear. I didn't meant to speak for others in that we\nshould or should not, and there might be other objections I don't recall\nor haven't heard.\n\nI seem to recall Peff wanting to use variadic macros at some point,\nalthough I can't recall specifically where. We also already use hacks\nto implement some of the features and hope that compilers will DTRT.\n\nAll the major compilers I'm aware of other than MSVC support C99, at\nleast well enough to do the things we'd likely end up doing.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"259091","messageId":"1428343320.3560.11.camel@ubuntu","threadId":"38995","inReplyTo":"551F8FC3.8010104@web.de","subject":"Re: Freeing struct lock_file?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-04-06T18:02:00Z","receivedAt":"2015-04-06T18:02:00Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Sat, 2015-04-04 at 09:16 +0200, Torsten Bögershausen wrote:\n> On 2015-04-04 02.24, David Turner wrote:\n> > On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:\n> >> David Turner writes:\n> >>\n> >>> Why is it impossible to free struct lock_files? I understand that they\n> >>> become part of a linked list, and that there's an atexit handler that\n> >>> goes over that list. But couldn't we just remove them from the linked\n> >>> list and then free them? \n> >>\n> >> I suspect that the code is worried about getting a signal, while it\n> >> is manipulating the linked list, and then cause the atexit handler\n> >> to walk a list that is in a broken state.\n> > \n> > This is technically possible, but practically unlikely: aligned\n> > pointer-sized writes are atomic on very nearly every processor, and that\n> > is all that is required to remove an item from a linked list safely in\n> > this case (though not, of course, in the general multi-threaded case).\n> > \n> > But I can see why git wouldn't want to depend on that behavior. C11 has\n> > a way to do this safely, but AIUI, git doesn't want to move to C99 let\n> > alone C11. So I guess this will just have to remain the way it is.\n> > \n> If you insist on using C11, may be.\n> \n> But if there is an implementation that is #ifdef'ed and only enabled for\n> \"known to work processors\" and a no-op for the others, why not ?\n> \n> Do you have anything in special in mind ?\n> A \"git diff\" may be a start for a patch series..\n\nI haven't written any code for this yet. I wanted to understand the\ncurrent code first.\n\nMy major worry is be that the code would be somewhat fragile as it\ndepends on not just the processor, but also the C compiler's structure\npacking rules, which are implementation-dependent. In practice, major\ncompilers' rules are safe, but it's annoying to have to depend on\n(especially since any bugs would be incredibly difficult to reproduce).\n"},{"id":"259109","messageId":"1428369163.3560.12.camel@ubuntu","threadId":"38995","inReplyTo":"xmqqfv8gg9bp.fsf@gitster.dls.corp.google.com","subject":"Re: Freeing struct lock_file?","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-04-07T01:12:43Z","receivedAt":"2015-04-07T01:12:43Z","isPatch":false,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2015-04-03 at 15:01 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > Why is it impossible to free struct lock_files? I understand that they\n> > become part of a linked list, and that there's an atexit handler that\n> > goes over that list. But couldn't we just remove them from the linked\n> > list and then free them? \n> \n> I suspect that the code is worried about getting a signal, while it\n> is manipulating the linked list, and then cause the atexit handler\n> to walk a list that is in a broken state.\n\nActually, couldn't we just block signals with sigprocmask while\nmanipulating the list?\n"}]}