D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3424 - Ref counting still doesn't work for std.stdio.File
Summary: Ref counting still doesn't work for std.stdio.File
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: Other Windows
: P2 critical
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-20 09:22 UTC by David Simcha
Modified: 2015-06-09 01:26 UTC (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description David Simcha 2009-10-20 09:22:29 UTC 
If this can't be fixed in short order it should at least be documented so that people don't rely on the ref counting and auto closing.  Here's a test case to show it doesn't work.  This fails on both Windows and Linux.

import std.stdio, std.file;

void main() {
    std.file.write("foo.txt", "stuff");
    foreach(i; 0..1_000_000) {
        openStuff();  // Throws too many files error rather quickly.
    }
}

void openStuff() {
    File myFile = File("foo.txt");
    // Not automatically closed.
}
Comment 1 Andrei Alexandrescu 2009-10-20 13:03:22 UTC 
(In reply to comment #0)
> If this can't be fixed in short order it should at least be documented so that
> people don't rely on the ref counting and auto closing.  Here's a test case to
> show it doesn't work.  This fails on both Windows and Linux.
> 
> import std.stdio, std.file;
> 
> void main() {
>     std.file.write("foo.txt", "stuff");
>     foreach(i; 0..1_000_000) {
>         openStuff();  // Throws too many files error rather quickly.
>     }
> }
> 
> void openStuff() {
>     File myFile = File("foo.txt");
>     // Not automatically closed.
> }

I'm glad to report I just fixed this and committed stdio.d.
Comment 2 Leandro Lucarella 2009-10-20 17:29:50 UTC 
Your commit, r1300, also removes some opSlice():

@@ -736,10 +740,4 @@
 
         /// Range primitive implementations.
-        ByLine!(Char, Terminator) opSlice()
-        {
-            return this;
-        }
-
-        /// Ditto
         bool empty() const
         {


Is this intended? If so, it would be nice if you could apply the changes in two different commits (if they are really unrelated, as I think they are; if not, please ignore this comment :).

I'm sorry to say this here I guess it's not the place, but I don't know of a better place.
Comment 3 Andrei Alexandrescu 2009-10-20 17:38:19 UTC 
(In reply to comment #2)
> Your commit, r1300, also removes some opSlice():
> 
> @@ -736,10 +740,4 @@
> 
>          /// Range primitive implementations.
> -        ByLine!(Char, Terminator) opSlice()
> -        {
> -            return this;
> -        }
> -
> -        /// Ditto
>          bool empty() const
>          {
> 
> 
> Is this intended? If so, it would be nice if you could apply the changes in two
> different commits (if they are really unrelated, as I think they are; if not,
> please ignore this comment :).
> 
> I'm sorry to say this here I guess it's not the place, but I don't know of a
> better place.

For a brief period, Walter required opSlice for foreach to work. He subsequently changed that, so I clean up that stuff when I get the opportunity.
Comment 4 Leandro Lucarella 2009-10-20 17:50:14 UTC 
(In reply to comment #3)
> (In reply to comment #2)
> > Your commit, r1300, also removes some opSlice():
> > 
> > @@ -736,10 +740,4 @@
> > 
> >          /// Range primitive implementations.
> > -        ByLine!(Char, Terminator) opSlice()
> > -        {
> > -            return this;
> > -        }
> > -
> > -        /// Ditto
> >          bool empty() const
> >          {
> > 
> > 
> > Is this intended? If so, it would be nice if you could apply the changes in two
> > different commits (if they are really unrelated, as I think they are; if not,
> > please ignore this comment :).
> > 
> > I'm sorry to say this here I guess it's not the place, but I don't know of a
> > better place.
> 
> For a brief period, Walter required opSlice for foreach to work. He
> subsequently changed that, so I clean up that stuff when I get the opportunity.

Ok, that comment would be an excellent commit message for a separated commit removing the opSlice() ;)

Thanks for the clarification.
Comment 5 Brad Roberts 2009-10-20 18:26:12 UTC 
The two @@BUG@@ comments that are still there.. is there still a bug?  If so, what is the bug number?