<AWT Dev> [8] Review request for 8027151: AWT_DnD/Basic_DnD/Automated/DnDMerlinQL/MultipleJVM failing on windows machine

[Oleg Pekhovskiy]

Hi Anthony,

I could refer to the fix for JDK-7075105. the finally {} block was added 
there and had never existed before.

Here's the way InputStream passed to DataTransferer.getInstance().
translateStream() gets closed:

In out case DataTransferer.translateStream() calls 
DataTransferer.translateStreamToInputStream() on line 1776 where
the stream is passed to ReencodingInputStream constructor. It is then 
passed to InputStreamReader constructor. Both classes override close() 
method where ReencodingInputStream closes referenced InputStreamReader and
InputStreamReader closes referenced InputStream.

ReencodingInputStream is returned by 
SunDropTargetContextPeer.getTransferData() method, called by 
DataFlavor.getReaderForText() that returns it to the client code.
So it's a client code obligation to call ReencodingInputStream.close().

This is the use case for the test mentioned in the issue description.
But in general we should investigate each use case to avoid memory 
leaks. As Petr pointed out it makes sense to do it for JDK 9, because it 
could be risky for now.

Thanks,
Oleg

On 29.10.2013 16:41, Anthony Petrov wrote:
> Hi Oleg,
>
> I'm not an expert in this code so I may ask some silly questions.
>
> I'm OK with the change #2.
>
> Regarding #1: could you please clarify what code is responsible for
> closing the stream now, after your fix? Is this documented/enforced
> anywhere (i.e. a finally{} block or something)? Have you run any
> regression tests to make sure this change doesn't introduce any memory
> leaks? Why was this not a problem before that we decided to fix this
> particular piece now, so late in the release?
>
> --
> best regards,
> Anthony
>
> On 10/28/2013 02:41 AM, Oleg Pekhovskiy wrote:
>> Hi all,
>>
>> please review the fix
>> http://cr.openjdk.java.net/~bagiras/8027151.1/
>> for
>> https://bugs.openjdk.java.net/browse/JDK-8027151
>>
>> This issue appeared after the changes made for JDK-7075105.
>> There were two problems:
>> 1. In drop target code, in SunDropTargetContextPeer.getTransferData()
>> method inputStream was closed in finally scope but was not yet used in
>> client code (indirectly). Just its reference stored for the future in
>> DataTransferer.getInstance().translateStream() call.
>>
>> 2. In drop target code, in DataTransferer.translateStream() there was no
>> String object handler (it was moved from
>> DataTransferer.translateBytesOrStream() only to
>> DataTransferer.translateBytes() method).
>>
>> Thanks,
>> Oleg