RE: Small ZipFile patch

[Jeroen Frijters]

Aaron M. Renn wrote:
> Jeroen Frijters (address@hidden) wrote:
> > > Jeroen Frijters (address@hidden) wrote:
> > > > BTW, the design of the 1.0 I/O classes is less than 
> > > stellar. Since the
> > > > FileDescriptor class is the owner of the native resource, 
> > > it should have
> > > > had a finalizer, instead of FileInputStream and 
> > > FileOutputStream (for
> > > > some reason RandomAccessFile doesn't have a finalizer). I 
> > > guess I should
> > > > be a bit mild in my criticism, finalizers weren't very well 
> > > understood
> > > > back then...
> > > 
> > > I am in the process of correcting this.
> > 
> > My gripe really wasn't directed at the Classpath 
> implementation, but at
> > the Sun implementation. How compatible do we want to be in 
> this case?
> > Compatible and broken or a little less compatible and not 
> so broken ;-)
> 
> I think we want to do the right thing.  I believe it was you yourself
> who pointed out a condition whereby a FileInputStream closing the file
> descriptor in finalization could potentially lead to 
> problems.  Particularly
> since you can use the getFD() call to return a valid file descriptor
> that would suddenly get un-valid if your FileInputStream happened to
> be garbage collected out from under you.  Better to let FileDescriptor
> handle its own finalization.  Actually, its much better if 
> the application
> manually closes the stream (which would close the underlying 
> file descriptor)
> because there is no guarantee that anything will ever get garbage
> collected.

Putting the finalize in FileDescriptor should be safe (once
FileDescriptor is an opaque object). That way you can be sure that a
FileDescriptor will only be closed by finalization if it isn't being
used anymore, even in the case of sharing. The only downside to this
would be that code developed under Classpath might behave erratically
when run under a Sun VM. I remember when I first started using Java
(beginning of 1997) I spent a couple of days tracking down a weird bug
that turned out to be caused by my (shared) FileDescriptor being closed
when I didn't expect it (because a temporary FileInputStream was being
finalized at some random point in time).

Regards,
Jeroen