Bug 12724

Summary:	fclose violates POSIX 2008 on seekable input streams
Product:	glibc	Reporter:	Eric Blake <eblake>
Component:	stdio	Assignee:	Not yet assigned to anyone <unassigned>
Status:	UNCONFIRMED ---
Severity:	normal	CC:	carlos, hannes, matz, toolchain
Priority:	P1
Version:	2.13
Target Milestone:	2.18
Host:		Target:
Build:		Last reconfirmed:

Description Eric Blake 2011-05-02 20:20:41 UTC

POSIX 2008 states:

"If the file is not already at EOF, and the file is one capable of seeking, the
file offset of the underlying open file description shall be adjusted so that
the next operation on the open file description deals with the byte after the
last one read from or written to the stream being closed."

[XSH fclose,
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fclose.html]

However, this sample program proves that for seekable input streams, glibc is
violating this requirement.  The same program runs to completion on Solaris 10.


#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <assert.h>

#define NAME "test-fclose.t"

int
main (void)
{
  const char buf[] = "hello world";
  int fd;
  int fd2;
  FILE *f;

  /* Prepare a seekable file.  */
  fd = open (NAME, O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (0 <= fd);
  assert (write (fd, buf, sizeof buf) == sizeof buf);
  assert (lseek (fd, 1, SEEK_SET) == 1);

  /* Create an output stream visiting the file; when it is closed, all
     other file descriptors visiting the file must see the new file
     position.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "w");
  assert (f);
  assert (fputc (buf[1], f) == buf[1]);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 2);

  /* Likewise for an input stream.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "r");
  assert (f);
  assert (fgetc (f) == buf[2]);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 3);

  /* Clean up.  */
  assert (remove (NAME) == 0);

  return 0;
}

Comment 1 Ulrich Drepper 2011-05-14 01:11:00 UTC

Fixed in git.

Comment 2 Eric Blake 2011-07-27 17:34:22 UTC

glibc 2.14 still has issues.  Per the latest wording:

http://austingroupbugs.net/view.php?id=87#c838

At page 805 line 26801 section fclose, change:

     the file offset of the underlying open file description shall be
     adjusted so that the next operation on the open file description
     deals with the byte after the last one read from or written to the
     stream being closed.

to:

     the file offset of the underlying open file description shall be
     set to the file position of the stream.

fclose() _must_ reposition the underlying fd, even if there was no last byte
read.  Therefore, this program, which passes on Solaris, shows that glibc is
still buggy:

#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <assert.h>

#define NAME "test-fclose.t"

int
main (void)
{
  const char buf[] = "hello world";
  int fd;
  int fd2;
  FILE *f;

  /* Prepare a seekable file.  */
  fd = open (NAME, O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (0 <= fd);
  assert (write (fd, buf, sizeof buf) == sizeof buf);
  assert (lseek (fd, 1, SEEK_SET) == 1);

  /* Create an output stream visiting the file; when it is closed, all
     other file descriptors visiting the file must see the new file
     position.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "w");
  assert (f);
  assert(lseek(fd, 4, SEEK_SET) == 4);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 4);

  /* Likewise for an input stream.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "r");
  assert (f);
  assert(lseek(fd, 4, SEEK_SET) == 4);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 4);

  /* Clean up.  */
  assert (remove (NAME) == 0);

  return 0;
}

Comment 3 Michael Matz 2011-08-12 15:31:21 UTC

Over at SuSE we ran into issues with this change.  See
  https://bugzilla.novell.com/show_bug.cgi?id=711829
.  Ruby does a dup/fdopen, but then doesn't do any reading/writing with 
the new stream, and when it gets fclosed the file position is reset also
for the other stream associated with the same file.  That one does
reads/writes.

I believe that's exactly the interaction between active/inactive streams
that http://austingroupbugs.net/view.php?id=87#c895 is talking about.

Now, that may or may not be useless code in ruby, but there we are.
The current half-POSIX-compliant way is worse than either the non-compliant
or the compliant way.

Comment 4 Carlos O'Donell 2012-11-29 15:19:04 UTC

We need someone to look into this on master and see if they can come up with a
patch to fix this.

Comment 5 Carlos O'Donell 2012-12-01 19:56:12 UTC

We are reverting this patch for 2.17 and will be fixing this correctly for 2.18
and ensuring that existing applications are not broken.