[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

Janne Blomqvist  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #11 from Janne Blomqvist  ---
Fixed on trunk, closing.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #10 from Janne Blomqvist  ---
Author: jb
Date: Sun Oct 16 06:28:15 2016
New Revision: 241211

URL: https://gcc.gnu.org/viewcvs?rev=241211=gcc=rev
Log:
PR 48587 Newunit allocator, cleanup

Improve error message, and remove a redundant check, as the same check is
done a bit earlier due to the PR 48587 patch.

2016-10-16  Janne Blomqvist  

PR libfortran/48587
* io/transfer.c (data_transfer_init): Improve error message,
remove redundant check.

Regtested on x86_64-pc-linux-gnu.

Modified:
trunk/libgfortran/ChangeLog
trunk/libgfortran/io/transfer.c

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #9 from Janne Blomqvist  ---
Author: jb
Date: Sat Oct 15 13:15:26 2016
New Revision: 241200

URL: https://gcc.gnu.org/viewcvs?rev=241200=gcc=rev
Log:
PR 48587 Newunit allocator

2016-10-15  Janne Blomqvist  

PR libfortran/48587
* gfortran.dg/negative_unit2.f90: New testcase.

Added:
trunk/gcc/testsuite/gfortran.dg/negative_unit2.f90

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #8 from Janne Blomqvist  ---
Author: jb
Date: Sat Oct 15 13:14:15 2016
New Revision: 241199

URL: https://gcc.gnu.org/viewcvs?rev=241199=gcc=rev
Log:
PR 48587 Newunit allocator

Currently GFortran newer reuses unit numbers allocated with NEWUNIT=,
instead having a simple counter that is decremented each time such a
unit is opened.  For a long running program which repeatedly opens
files with NEWUNIT= and closes them, the counter can wrap around and
cause an abort.  This patch replaces the counter with an allocator
that keeps track of which units numbers are allocated, and can reuse
them once they have been deallocated.  Since operating systems tend to
limit the number of simultaneous open files for a process to a
relatively modest number, a relatively simple approach with a linear
scan through an array suffices.  Though as a small optimization there
is a low water indicator keeping track of the index for which all unit
numbers below are already allocated.  This linear scan also ensures
that we always allocate the smallest available unit number.

2016-10-15  Janne Blomqvist  

PR libfortran/48587
* io/io.h (get_unique_unit_number): Remove prototype.
(newunit_alloc): New prototype.
* io/open.c (st_open): Call newunit_alloc.
* io/unit.c (newunits,newunit_size,newunit_lwi): New static
variables.
(GFC_FIRST_NEWUNIT): Rename to NEWUNIT_START.
(next_available_newunit): Remove variable.
(get_unit): Call newunit_alloc, don't try to create negative
external unit.
(close_unit_1): Call newunit_free.
(close_units): Free newunits array.
(get_unique_number): Remove function.
(newunit_alloc): New function.
(newunit_free): New function.
* io/transfer.c (data_transfer_init): Check for invalid unit
number.

testsuite ChangeLog:

2016-10-15  Janne Blomqvist  

PR libfortran/48587
* gfortran.dg/negative_unit2.f90: New testcase.

Modified:
trunk/gcc/testsuite/ChangeLog
trunk/libgfortran/ChangeLog
trunk/libgfortran/io/io.h
trunk/libgfortran/io/open.c
trunk/libgfortran/io/transfer.c
trunk/libgfortran/io/unit.c

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

Janne Blomqvist jb at gcc dot gnu.org changed:

   What|Removed |Added

   Assignee|unassigned at gcc dot gnu.org  |jb at gcc dot gnu.org

--- Comment #7 from Janne Blomqvist jb at gcc dot gnu.org ---
I have a preliminary patch, which I'm planning to finish and submit when 4.10
stage 1 opens. Assigning to myself.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[fxcoudert at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

Francois-Xavier Coudert fxcoudert at gcc dot gnu.org changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2011.04.19 13:40:08
 CC||fxcoudert at gcc dot
   ||gnu.org
 Ever Confirmed|0   |1

--- Comment #6 from Francois-Xavier Coudert fxcoudert at gcc dot gnu.org 
2011-04-19 13:40:08 UTC ---
(In reply to comment #4)
 estimates around 10 hours. I suspect current trunk and changing the code to 
 use
 NEWUNIT= are in the same ballpark.

With a tmpfs filesystem, I get that down to 1.5 hour. Might be worth fixing at
some point.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #5 from Janne Blomqvist jb at gcc dot gnu.org 2011-04-15 09:13:14 
UTC ---
Reading the standard, my simple suggestion in #1 will not actually work. One
reason is that the NEWUNIT value may not be -1; with my approach this is
possible if one first closes fd 1. Secondly, it's possible to reassociate also
a negative unit number with a new file; but there is not guarantee that the
kernel will assign the same file descriptor to the new file. Wrt. I found a bug
in the current implementation, see PR 48618.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #4 from Janne Blomqvist jb at gcc dot gnu.org 2011-04-14 08:12:33 
UTC ---
(In reply to comment #3)
 I suppose one could put an OPEN .. CLOSE in a DO LOOP and see what happens 
 now.

With gfortran 4.4.3 on Linux 2.6.32, /tmp on ext4, single SATA disk, and 2.67
GHz Xeon X3450 CPU, the following program

program twobfiles
  implicit none
  integer, parameter :: dp = kind(1.0d0)
  integer :: ii, cnt, cnt2, cnt_rate
  real(dp) :: elapsed
  call system_clock(cnt, cnt_rate)
  do ii = 10, 10010
 open(unit=ii, status=scratch)
 close(ii)
  end do
  call system_clock(cnt2)
  elapsed = real((cnt2 - cnt), dp) / cnt_rate
  print *, Estimate for opening and closing 2 billion files: , 
   elapsed * 2._dp**31 / 1,  s.
end program twobfiles

estimates around 10 hours. I suspect current trunk and changing the code to use
NEWUNIT= are in the same ballpark.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

Tobias Burnus burnus at gcc dot gnu.org changed:

   What|Removed |Added

 CC||burnus at gcc dot gnu.org

--- Comment #1 from Tobias Burnus burnus at gcc dot gnu.org 2011-04-13 
10:20:00 UTC ---
There is a problem with reusing numbers - it is probably only a small one - but
it might be larger than the problem of running out of unit IDs:

The user might reuse NEWUNIT= IDs:

Open(newunit=id)
...
close(id)
open(unit=id) ...

The latter is illegal as one may only pass positive numbers to unit=, but I
would not count on it. Given that the number is huge, we decided that the
current method of just incrementing the value should be sufficient for nearly
all cases and one can think about something clever if a real-space code
encounters the problem. However, I don't mind if one solves the problem before.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

--- Comment #2 from Janne Blomqvist jb at gcc dot gnu.org 2011-04-13 10:46:42 
UTC ---
(In reply to comment #1)
 There is a problem with reusing numbers - it is probably only a small one - 
 but
 it might be larger than the problem of running out of unit IDs:
 
 The user might reuse NEWUNIT= IDs:
 
 Open(newunit=id)
 ...
 close(id)
 open(unit=id) ...
 
 The latter is illegal as one may only pass positive numbers to unit=, but I
 would not count on it.

I don't see how this would differ from the current behavior vs. my suggested
implementation. In any case, we check that the UNIT= number is positive when
opening units:

program negative_unit
  implicit none
  open(-10, file=foo.txt)
  write(-10, *) Hello world
  close(-10)
end program negative_unit

$ ./negative_unit 
At line 3 of file negative_unit.f90
Fortran runtime error: Bad unit number in OPEN statement

(this is with 4.4; presumably this restriction hasn't been lifted?)

Both the current implementation and my suggested one ensure that any unit
number returned in NEWUNIT= is negative. This ensures that the UNIT= and
NEWUNIT= number ranges are kept separate from each other, so they shouldn't
interact.

 Given that the number is huge, we decided that the
 current method of just incrementing the value should be sufficient for nearly
 all cases and one can think about something clever if a real-space code
 encounters the problem.

Ah, but what about a k-space code? ;)

 However, I don't mind if one solves the problem before.

Yeah, well, I don't think this is a big deal; after all it's pretty unlikely
that any program will overflow the counter.

[Bug libfortran/48587] Avoid exhausting unit number with NEWUNIT=

[jvdelisle at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48587

Jerry DeLisle jvdelisle at gcc dot gnu.org changed:

   What|Removed |Added

 CC||jvdelisle at gcc dot
   ||gnu.org

--- Comment #3 from Jerry DeLisle jvdelisle at gcc dot gnu.org 2011-04-13 
18:58:47 UTC ---
If someone overflows the counter I think they should get what they deserve.  ;)

I suppose one could put an OPEN .. CLOSE in a DO LOOP and see what happens now.
It might take a while to get there. IIRC when I implemented this I set the
limit small and tested an error check for this, but maybe I am wrong.