Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define close method? #5

Closed
quinnj opened this issue Oct 3, 2015 · 13 comments
Closed

Define close method? #5

quinnj opened this issue Oct 3, 2015 · 13 comments

Comments

@quinnj
Copy link

quinnj commented Oct 3, 2015

I often use Libz streams in situations where I might be dealing with a regular IOStream or a compressed stream, which works pretty well, except I typically want to call close(::IO) to close my stream. I'm not sure if close is applicable to a Libz stream, but maybe we could define a method anyway for the IO interface?

@sbromberger
Copy link

Example:

julia> io = open("pgp2.jgz","r") |> ZlibInflateInputStream;

julia> close(io)
ERROR: MethodError: `close` has no method matching close(::BufferedStreams.BufferedInputStream{Libz.Source{:inflate,BufferedStreams.BufferedInputStream{IOStream}}})

@quinnj
Copy link
Author

quinnj commented Dec 9, 2015

@TransGirlCodes
Copy link
Member

I'm not sure I understand what that does in CSV.jl - it accepts a stream and returns nothing?

@bicycle1885
Copy link
Member

Since a Source is automatically closed by a finalizer after it is not reachable, there is actually no need to call the close method of a compressed stream. But people who expects compressed streams to be an IO-like object would like to call close like IOStream. I think it is reasonable to define a close method for the BufferedInputStream type that does nothing except returning a nothing.

@TransGirlCodes
Copy link
Member

Ah I see! so it's closed regardless, but the nothing returning method does nothing if a user calls it, rather than resulting in a program crash due to no method error?

@bicycle1885
Copy link
Member

Yes, I think so. Just defining a close method for the BufferedInputStream that does nothing will solve the no method error problem. Another option is to eagerly close the underlying stream of a buffered stream and immediately release the resource when close is called, this may need a little bit more care.

@quinnj
Copy link
Author

quinnj commented Dec 9, 2015

Yes, I have methods that take an IO object and sometimes that may be a IOStream or it might be a compressed stream. I need to close the IOStream objects. We don't have official "interfaces" for IO objects, but I'm pretty sure close would be one (unless we decide that all IO objects should rely on finalizers).

@TransGirlCodes
Copy link
Member

I've read some discussion of this today, aren't finalisers suboptimal? Something about the resources not being released even some time after GC.

@quinnj
Copy link
Author

quinnj commented Dec 9, 2015

There's no perfect solution right now (see the discussions here: JuliaLang/julia#11207), but I do think that finalizers are the best way to go, because they ensure things get torn down eventually, and the user can call finalize(obj) to force finalization.

@bicycle1885
Copy link
Member

Having both finalizers and closing methods would be ideal. I think the close method is mandatory since there may be people who want to open a lot of files and immediately close it after checking something.

@TransGirlCodes
Copy link
Member

So (and maybe this is stupid, I'm not sure, only just getting caught up on the discussion) is the answer then to have finializers for this, but then if close is called, to explicitly call the finalize method?

@quinnj
Copy link
Author

quinnj commented Dec 10, 2015

I think ideally, one would always use finalizers and be able to call the finalize method (as currently exists in base julia). The current problems are that finalize isn't extremely efficient and it's not totally predictable when a finalizer will run. This is compared with Python's with statement or Go's defer statement that indicate that at the end of the current scope, finalizers should be run.

@bicycle1885
Copy link
Member

This is closed by #21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants