Skip to content

fixes purescript/#1673#1674

Merged
garyb merged 3 commits into
purescript:masterfrom
mgmeier:master
Dec 5, 2015
Merged

fixes purescript/#1673#1674
garyb merged 3 commits into
purescript:masterfrom
mgmeier:master

Conversation

@mgmeier

@mgmeier mgmeier commented Nov 25, 2015

Copy link
Copy Markdown
Contributor

Among other solutions

  • using a different data type (e.g Data.Text)
  • using strict I/O (package strict-io)

this fix is the least invasive.

It's tested on my system and works. It forces readFile to advance till reaching EOF (or an exception) and change the corresponding file handle's state, so that it can be GC'd.

@paf31

paf31 commented Nov 25, 2015

Copy link
Copy Markdown
Contributor

Neat, thanks! 👍

Perhaps it makes sense to put this in its own function, but could you please document it with a line comment in either case?

@hdgarrood

hdgarrood commented Nov 25, 2015

Copy link
Copy Markdown
Contributor

As an aside, I think switching to Text is probably a good idea, and I think there's an issue open for it somewhere anyway? #547? Not sure if that's related.

@mgmeier

mgmeier commented Nov 25, 2015

Copy link
Copy Markdown
Contributor Author

comment added.
IMHO it's preferable to leave length js in the same closure as js <- readFile ..., so we prevent leaking of lazily read content in any case. This also guarantees the file handle being marked as "semi-closed" by the Haskell runtime before the next iteration, so it doesn't interfere with Mac OS 's crappy resource management.

@mgmeier

mgmeier commented Nov 25, 2015

Copy link
Copy Markdown
Contributor Author

and yeah, +1 for switching to Text ;)

@paf31

paf31 commented Nov 30, 2015

Copy link
Copy Markdown
Contributor

Could you please include yourself in https://github.com/purescript/purescript/blob/master/CONTRIBUTORS.md?

@mgmeier

mgmeier commented Dec 1, 2015

Copy link
Copy Markdown
Contributor Author

✔️

garyb added a commit that referenced this pull request Dec 5, 2015
@garyb
Merge pull request #1674 from mgmeier/master
4579b16 
fixes purescript/#1673
@garyb garyb merged commit 4579b16 into purescript:master Dec 5, 2015
@garyb

garyb commented Dec 5, 2015

Copy link
Copy Markdown
Member

Thanks! Sorry about the delay in merging, Phil is busy at the moment and I hadn't been keeping track of this.

@garyb garyb mentioned this pull request Dec 5, 2015
Closed
@mgmeier

mgmeier commented Dec 6, 2015

Copy link
Copy Markdown
Contributor Author

no problem. looking forward to the next release 👍

@chrisdone chrisdone mentioned this pull request Sep 29, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mgmeier @paf31 @hdgarrood @garyb