RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

[Langer, Christoph]

Hi Dmitry, Thomas,

Dmitry, I think you are referring to an outdated version of the webrev, the current one is this:
http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.01/webrev/

However, I agree - the lock should probably not be taken every time but only in the case where we find the entry table was not yet allocated.

So, maybe getFdEntry should always do this:
entryTable = fdTable[rootArrayIndex]; // no matter if rootArrayIndex is 0

Then check if entryTable is NULL and if yes then enter a guarded section which does the allocation and before that checks if another thread did it already.

Also I'm wondering if the entryArrayMask and the rootArrayMask should be calculated once in the init() function and stored in a static field? Because right now it is calculated every time getFdEntry() is called and I don't think this would be optimized by inlining...

Best regards
Christoph

-----Original Message-----
From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On Behalf Of Dmitry Samersoff
Sent: Dienstag, 1. März 2016 11:20
To: Thomas Stüfe <thomas.stuefe at gmail.com>; Java Core Libs <core-libs-dev at openjdk.java.net>
Subject: Re: RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

Thomas,

Sorry for being later.

I'm not sure we should take a lock at ll. 131 for each fdTable lookup.

As soon as we never deallocate fdTable[base_index] it's safe to try to
return value first and then take a slow path (take a lock and check
fdTable[base_index] again)

-Dmitry


On 2016-02-24 20:30, 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:
> http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.00/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
> description.
> 
> 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 RLIMIT_NO_FILE.
> 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 for
> every call to getFdEntry() - I do not think this matters, as this is all in
> preparation for an IO system call, which are usually way more expensive
> than a pthread mutex. But again, this could be optimized.
> 
> 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
> too.
> 
> Thank you and Kind Regards, Thomas
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.