From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23848 invoked by alias); 29 Sep 2014 10:59:24 -0000 Mailing-List: contact zsh-users-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Users List List-Post: List-Help: X-Seq: 19183 Received: (qmail 27920 invoked from network); 29 Sep 2014 10:59:12 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 X-AuditID: cbfec7f4-b7f156d0000063c7-23-54293b769649 Date: Mon, 29 Sep 2014 11:59:01 +0100 From: Peter Stephenson To: zsh-users@zsh.org Subject: Re: File descriptor leakage? Message-id: <20140929115901.255bacb9@pwslap01u.europe.root.pri> In-reply-to: <140928230710.ZM5202@torch.brasslantern.com> References: <20140929035338.GA29747@lilyforest> <140928230710.ZM5202@torch.brasslantern.com> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuphluLIzCtJLcpLzFFi42I5/e/4Zd0ya80Qg/v3eC12nFzJ6MDoserg B6YAxigum5TUnMyy1CJ9uwSujHXzpjEXzBSs2DVxI2MD4xreLkZODgkBE4nH668zQthiEhfu rWfrYuTiEBJYyigxq38eK5TDJHFn2TvmLkYODhYBVYlDe/1BGtgEDCWmbpoN1iwiICqxfMVm dhBbGKhk68UWNhCbV8BeYsuxRWA1nAKWEp92bgaLCwnESVw8c5wJxOYX0Je4+vcTE8QR9hIz r5xhhOgVlPgx+R4LiM0soCWxeVsTK4QtL7F5zVvmCYwCs5CUzUJSNgtJ2QJG5lWMoqmlyQXF Sem5hnrFibnFpXnpesn5uZsYISH4ZQfj4mNWhxgFOBiVeHg5VmiECLEmlhVX5h5ilOBgVhLh /aCmGSLEm5JYWZValB9fVJqTWnyIkYmDU6qBMYArvVyVK5Ch+5+T4RQHMaWJQkUfzhlxyids qHP/fu2qZ+Odvaf+NXx/lc7eeJ578c2KyONZT3fneotcZpy2eecbCZdCwZueJvabE7627jvx YVfP4h1i69NcuvuXHmYMn5wjmidm+PFTh2XVowuFO+wmvQqJ7b85ZdJWj58HGK023z9dazIj TImlOCPRUIu5qDgRAOSNh2UfAgAA On Sun, 28 Sep 2014 23:07:10 -0700 Bart Schaefer wrote: > On Sep 29, 11:53am, lilydjwg wrote: > } > } I discover that my zsh keeps a file descriptor to a deleted > } ".histfile.new" file: > > This is the file created when the HIST_SAVE_BY_COPY option is set, which > is then renamed to the name given by $HISTFILE after being written. I > don't know of any reason the behavior for this would have changed from > 5.0.5 to 5.0.6, but it's possible that workers/33116 is related. The option is on by default, so a lot of people ought to be seeing this. It looks like either it's system-specific or maybe to do with some hairy history option combination --- there are plenty of those, though it's not obvious why they'd have an effect here. > There is an assumption that > > fd = open(file, O_WRONLY, 0600); > out = fdopen(fd, "w"); > fclose(out); > > will close fd, but that assumption has been there for as long as the > HIST_SAVE_BY_COPY option has existed. Fairly easy to add a "close(fileno(out));" and see if it goes away. That would be after the fclose() currently at line 2682 of Src/hist.c. I note we don't handle the case where the open() succeeded but the fdopen() failed in this case (around line 2595) unlike other cases (around 2822 and 2876). This would probably be good practice anyway. lilydjwg wrote: > Hi, I did a git bisect and found this commit is the first bad one: > 979f72199f3d3e98b1f6c712fa26f731dac2cb51 (...finds out about "git show" again...) Hmm, that's to do with file locking. That might suggest some path through the code where this isn't handled that neither Bart nor I have spotted yet. diff --git a/Src/hist.c b/Src/hist.c index d29a65a..4660fd0 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -2593,7 +2593,12 @@ savehistfile(char *fn, int err, int writeflags) out = NULL; } else { int fd = open(tmpfile, O_CREAT | O_WRONLY | O_EXCL, 0600); - out = fd >= 0 ? fdopen(fd, "w") : NULL; + if (fd >=0) { + out = fdopen(fd, "w"); + if (!out) + close(fd); + } else + out = NULL; } #ifdef HAVE_FCHMOD pws