On Mon, 2013-07-29 at 14:58 +0400, Andrew Brygin wrote:
> Hello,
> 
> could you please review a fix for CR 8020983?
> 
> Bug: http://bugs.sun.com/view_bug.do?bug_id=8020983
> Webrev: http://cr.openjdk.java.net/~bae/8020983/8/webrev.00/
> 
> This fix replaces the strong reference in the streamBuffer structure
> with a weak reference to prevent locking a reader/writer instance.
> 
> A macro GET_STREAM_REF is introduced to obtain a reference to the
> 'stream' with checks whether the reference is still alive. If not,
> the plugin instance is terminated. However, this case seems to be
> very unlikely because we need an alive plugin instance to run into
> these methods.
> 
> Supplied regression test verifies that writer instances can be
> collected.
> 
> Please take a look.

Hi Andrew,

I'm not a reviewer, but the fix looks good for me.

One suggestion though, since streamBufferStruct->streamRef is a source
of confusion.

typedef struct streamBufferStruct {
  jweak streamRef; // ImageInputStream or ImageOutputStream

but unless I'm completely blind doesn't hold the input/output stream any
longer. It actually contains JPEGImageReader/Writer references directly.

It may be worth to update the comment as well while we are there to
avoid confusion (this is why in our previous fix we didn't bother with
the weak references, but yours is definitely a nicer fix).

Cheers,
Mario


Reply via email to