RFR: JDK-8033917 Keep track of file paths in file streams and channels for instrumentation purposes

[Dmitry Samersoff]

On 2014-02-07 19:32, Chris Hegarty wrote:
> On 07/02/14 15:24, Dmitry Samersoff wrote:
>> Staffan,
>>
>> FileInputStream.java
>>
>> 55:  It's better to initialize path with null.
> 
> I'm afraid I disagree with this. The default value is already null, why
> set it to null again? I see this pattern all over the code, but it seems
> completely redundant to me.

Yes, It's NOOP but it makes readers and variety of "security" tools happy.

I will not press for it, but as far as rest of the code

(e.g. private FileChannel channel = null; )

uses this pattern and initialize variables explicitly, I think it's good
to initialize this variable as well.

-Dmitry


> 
> -Chris.
> 
>> 134: It's better to assign name at one of first lines, in this case we
>> will be able to retrieve file name ever if open fails for some reason.
>> 171: It's not necessary
>>
>> (the same is applicable to other files)
>>
>> I'm a bit scared changing signature of public methods of FileChannelImpl
>> but if Alan says it's OK - lets go with it.
>>
>> -Dmitry
>>
>>
>> On 2014-02-07 16:07, Staffan Larsen wrote:
>>> Instrumentation agents that want to instrument
>>> FileInputStream/FileOutputStream to see which files are being
>>> accessed do not currently have access to the file system path of the
>>> stream. This is because the path is never stored in the stream class,
>>> only the file descriptor is. (This is also true for RandomAccessFile
>>> and FileChannel).
>>>
>>> An agent could instrument the respective constructors to store the
>>> path. The problem is where to store it. To add a field, the
>>> instrumentation agent needs to change the schema of the class. This
>>> is not possible at runtime but can be done at class-loading time.
>>> However for a j.l.instrument agent these classes are already defined
>>> when the agent is first called. For a native JVMTI agent the problem
>>> becomes parsing and modifying byte codes in a native agent which is
>>> error prone and requires a lot of code to maintain.
>>>
>>> If instead the stream classes were modified to store a reference to
>>> the path, it would be readily available for agents at a minimum of
>>> cost to the libraries. This is what this patch does. FileInputStream,
>>> FileOutputStream, RandomAccessFile and FileChannelImpl are modified
>>> to record the path they operate on in a private field. There are no
>>> accessors added to retrieve the path - it is purely stored for
>>> instrumentation purposes. The path is intentionally not resolved to
>>> be an absolute path since that would potentially add unwanted
>>> overhead. If a stream is created from a file descriptor, no path will
>>> be stored.
>>>
>>> The overhead for this path will be keeping the path String alive for
>>> a longer period of time. I hope this will not cause any problems.
>>>
>>> A consumer of this feature will be Java Flight Recorder, but the
>>> implementation is usable by other agents as well.
>>>
>>> webrev: http://cr.openjdk.java.net/~sla/8033917/webrev.00/ bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8033917
>>>
>>> Thanks, /Staffan
>>>
>>
>>


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