From: Philip Oakley Date: Fri, 01 Apr 2016 07:26:18 GMT Subject: Re: [PATCHv2 3/4] bundle: don't leak an fd in case of early return Message-ID: <55AD693FD6AC44119B8368ABC36FB1A9@PhilipOakley> In-Reply-To: From: "Stefan Beller" > On Thu, Mar 31, 2016 at 12:00 PM, Philip Oakley > wrote: >> From: "Stefan Beller" >>> >>> In successful operation `write_pack_data` will close the `bundle_fd`, >>> but when we exit early, we need to take care of the file descriptor >>> as well as the lock file ourselves. The lock file may be deleted at the >>> end of running the program, but we are in library code, so we should >>> not rely on that. >>> >>> Helped-by: Jeff King >>> Signed-off-by: Stefan Beller >> >> >> Has this been tested on Windows? I had a similar problem very recently >> https://groups.google.com/forum/#!msg/git-for-windows/6LPxf9xZKhI/-s7XD18yCwAJ >> where a bad rev-list-arg would cause the `bundle create` to die, and, on >> windows, leave the incomplete bundle file locked. >> >> dscho suggested one possible solution for that, but I haven't had any >> time >> to try any patches. > > I think with Jeffs suggestion to only rollback the lock in case of > (!bundle_to_stdout) > we're not making it worse. I do not have a Windows machine at hand to test > it :( Thant's fine. I just wanted to make folks aware of the problem so it doesn't get worse. It looks like the cause is that the bundle.c:compute_and_write_prerequisites(), which includes parsing the arg-list (and die on bad arg), is called after the creation and locking of the bundle file descriptor and header, thus leaving it hanging on Windows. -- Philip