RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all
thomas.stuefe at gmail.com
Thu Feb 25 16:51:54 UTC 2016
thank you for the review!
Please find my comments inline.
On Wed, Feb 24, 2016 at 8:28 PM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> Hi Thomas,
> On 2/24/2016 12:30 PM, Thomas Stüfe wrote:
>> Hi all,
>> please take a look at this proposed fix.
>> The bug: https://bugs.openjdk.java.net/browse/JDK-8150460
>> The Webrev:
>> Basically, the file descriptor table implemented in linux_close.c may not
>> work for RLIMIT_NO_FILE=infinite or may grow very large (I saw a 50MB
>> table) for high values for RLIMIT_NO_FILE. Please see details in the bug
>> The proposed solution is to implement the file descriptor table not as
>> plain array, but as a twodimensional sparse array, which grows on demand.
>> This keeps the memory footprint small and fixes the corner cases described
>> in the bug description.
>> Please note that the implemented solution is kept simple, at the cost of
>> somewhat higher (some kb) memory footprint for low values of
>> This can be optimized, if we even think it is worth the trouble.
>> Please also note that the proposed implementation now uses a mutex lock
>> every call to getFdEntry() - I do not think this matters, as this is all
>> preparation for an IO system call, which are usually way more expensive
>> than a pthread mutex. But again, this could be optimized.
> I would suggest preallocating the index array and then skip the mutex
> for that case.
> That would give the same as current performance.
I did this.
> And I'd suggest a different hi/low split of the indexes to reduce the size
> of pre-allocated memory.
> Most processes are going to use a lot fewer than 16384 fd's. How about
I did this too. Now I calculate the split point based on RLIMIT_NO_FILE.
For small values of RLIMIT_NO_FILE
(<64K), I effectivly fall back to a one-dimensional array by making the
first level table a size 1. For larger values,
multiple second level tables, each 64K size, will be allocated on demand
(save for the first one which is preallocated).
> I have my doubts about needing to cover fd's up to the full range of 32
> Can the RLIMIT_NO_FILE be used too parametrize the allocation of the first
> level table?
I did this.
Interesting note, I found nowhere in the Posix specs a mentioning that
socked descriptors have to be handed out
sequentially and therefore cannot be larger than RLIMIT_NO_FILE. But in
reality on all operating systems file descriptors
seem to be [0, RLIMIT_NO_FILE).
Not specific to your change but it would nice to see consistency between
> libnio and libnet on
> the name of the sigWakeup/INTERRUPT_SIGNAL constant.
I agree, but this is out of the scope of this bug fix.
>> This is an implementation proposal for Linux; the same code found its way
>> to BSD and AIX. Should you approve of this fix, I will modify those files
> yes please.
> $.02, Roger
>> Thank you and Kind Regards, Thomas
More information about the core-libs-dev