From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/7882 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Making stdio writes robust/recoverable under errors Date: Fri, 5 Jun 2015 17:14:01 -0400 Message-ID: <20150605211401.GW17573@brightrain.aerifal.cx> References: <20150529135300.GD17573@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1433538876 29239 80.91.229.3 (5 Jun 2015 21:14:36 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 5 Jun 2015 21:14:36 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-7895-gllmg-musl=m.gmane.org@lists.openwall.com Fri Jun 05 23:14:36 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Z0ywS-00087w-Lq for gllmg-musl@m.gmane.org; Fri, 05 Jun 2015 23:14:16 +0200 Original-Received: (qmail 25714 invoked by uid 550); 5 Jun 2015 21:14:14 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 25691 invoked from network); 5 Jun 2015 21:14:14 -0000 Content-Disposition: inline In-Reply-To: <20150529135300.GD17573@brightrain.aerifal.cx> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:7882 Archived-At: On Fri, May 29, 2015 at 09:53:00AM -0400, Rich Felker wrote: > Currently, musl's stdio write operations take the liberty of leaving > the current position and actual contents written rather unpredictable > if a write error occurs. Originally the motivation was probably a mix > of uncertainty about what to do and lack of code (or a desire to avoid > complexity) for tracking what part of the buffer was unwritten if a > write error occurred before the buffer was written out. But commit > 58165923890865a6ac042fafce13f440ee986fd9, as part of adding > cancellation support, added the code to track what part of the buffer > remains unwritten, and therefore, from what I can see, avoiding data > loss when stdio writes fail transiently (like EINTR, for instance) is > simply a matter of removing line 31 in the error path of > __stdio_write: > > f->wpos = f->wbase = f->wend = 0; > > so that the buffer is not thrown away. > > An attached test program demonstrates the issue. It assumes Linux 64k > pipe buffers so it's not appropriate for inclusion in tests in its > current form, but it gets the job done where it works. It attempts to > send 128k over a pipe via write followed by a series of fwrite retries > that are interrupted by signals that cause EINTR; thanks to the > kernel's pipe buffer semantics, there's basically no timing/race > aspect involved. With current musl, I get only 130560 across the pipe; > with the above line removed, I get all 131072. > > I think this is a good change to make from the standpoint of > robustness. Even if most errors are not recoverable, some may be; in > fact ENOSPC could be handled meaningfully if the calling program knows > it has temp files it could delete. What I am concerned about, though, > are residual assumptions that, after f->write, the write buffer will > be clear. With this change, fflush and similar operations would no > longer discard the buffer on error, meaning we would need to check > that callers don't need that property. As another example, fseek fails > when the buffer can't be written out, but since the current buffer > write throws away the buffer on failure, a second fseek call will > succeed; this change would leave it failing. (As an aside, rewind > already fails in this case and throws away the error; this is almost > certainly wrong. It should probably attempt to flush and throw away > the buffer on failure before seeking so that the seek always > succeeds.) > > I don't want to make these changes right now at the end of a release > cycle but I might promote them to important targets for the next > cycle. I'm a bit undecided on whether to move forward with this or not. While there's some benefit to being able to resume/recover transient errors that occur during buffered writing, there are also disadvantages: a stream that has buffered unwritable output due to error is stuck in a permanent state of being unable to seek, switch to reading, etc. I don't think the EINTR case is very interesting; using EINTR to interrupt stdio writes is not viable anyway since, if some data was already written, the underlying write will return with a short write and stdio will re-block trying to write the rest. There is one interesting case: in the event of ENOSCP, a caller could remove some temp files and then attempt to resume writing. On the other hand, the permanent "no space" condition that would result for fmemopen is rather ugly/unfortunate, because this condition "should" be caught before the data is buffered, but isn't. A similar condition exists for open_[w]memstream, but those would only error out for "no space" when malloc fails. I'm still not clear on which behavior is best, but probably going to put aside ideas of changing this for now unless good reasons come to light from discussion. Rich