Hi Mario,
thanks for the comments!
I agree that 'stream' and 'streamRef' are not appropriate names now,
because these entities point to instances of writer/reader.
However, these entities use the writer/reader only as a holder of
certain set of I/O routines, so probably names for these entities
should reflect their I/O nature.
Would 'ioRef' and 'input'/'output' sound better than 'stream'?
Thanks,
Andrew
On 7/29/2013 3:48 PM, Mario Torre wrote:
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
