That case basically just re-wraps that fd with a new IO object. In MRI
it *can* be prematurely closed if you have multiple IOs pointing at
the same fd, and in JRuby it's no different (though in our case, it's
because unlike reopen, IO.new doesn't dup the descriptor, and so we
don't increment our reference count).
So think that case is ok as well.
I'm going to push this change and see how things go. We're going into
an RC cycle anyway, so if it causes trouble hopefully it will get
caught there. But I would wager it's going to be fine (provided that
all IO wrapper objects clean up those resources appropriately, which
by now they sure better).
- Charlie
On Sun, Mar 7, 2010 at 9:38 AM, Thomas E Enebo <tom.en...@gmail.com> wrote:
> If there is no compatibility problem with doing this then so much the
> better. I thought it had something to do with reattaching to the same
> IO resource when you opened IO in weird ways like using fileno to IO
> constructor? something like (not tested just remembered):
>
> a = File.new("foo")
>
> b = IO.new(a.fileno)
>
> I thought these were both supposed to have the same underlying IO crud?
>
> -Tom
>
> On Sun, Mar 7, 2010 at 7:58 AM, Charles Oliver Nutter
> <head...@headius.com> wrote:
>> I'm pretty well convinced now that there's no reason we should be
>> maintaining a weak map from fileno numbers (our pseudo-file
>> descriptors) to ChannelDescriptor objects (or file-descriptor-like
>> wrapper around a NIO Channel). Here's why:
>>
>> * If you open a file in C and never close it, it will never close
>> until the process terminates. In JRuby, if you sysopen a file and
>> never close it, we explicitly hold a hard reference to the channel to
>> make sure it doesn't get closed, only because normally we put such
>> mappings into a weak map.
>> * If you open a file in Ruby and never close it and never dereference
>> it, it will stay open. JRuby matches this behavior, since we have a
>> hard reference to the channel via the RubyFile object.
>> * If you open a file in Ruby and never close it and dereference it, it
>> will eventually GC and finalize and close. JRuby also matches this
>> behavior, since IO objects have finalization associated with them.
>> * If you reopen a fileno in Ruby with a new IO object, it may or may
>> not be kept open if the original file closes. Ruby has very inexact
>> behavior regarding the lifetime of file descriptors that have been
>> reopened. In JRuby, if you reopen a file descriptor, we increment a
>> reference count. ChannelDescriptor will only close if it finalizes
>> (becomes dereferenced everywhere) or if its reference count falls to
>> zero (all wrapping IO objects have released it or themselves been
>> collected).
>>
>> I have been unable to find any case where we'd hold references longer
>> than necessary if we removed the weak association from fileno to
>> ChannelDescriptor, and the semantics of file descriptors seems to fit
>> this assumption: they stay open unless closed or unless the objects
>> referencing them get GCed.
>>
>> I also suspect this weak map is the source of some of our spurious
>> EBADF errors, for tests that work with fileno values directly.
>>
>> I've attached a patch for this. All tests run successfully with it,
>> though that's not really an indication that it's "good" since the only
>> likely problem we'd see is leaking of ChannelDescriptor/Channel
>> objects.
>>
>> Eliminating the weak map simplifies IO logic and would reduce the
>> number of weak references we have in-flight for IO-heavy applications.
>>
>> What sayeth you all?
>>
>> - Charlie
>>
>> ---------------------------------------------------------------------
>> To unsubscribe from this list, please visit:
>>
>> http://xircles.codehaus.org/manage_email
>>
>>
>
>
>
> --
> blog: http://blog.enebo.com twitter: tom_enebo
> mail: tom.en...@gmail.com
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
> http://xircles.codehaus.org/manage_email
>
>
>
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
