[OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while non fully initialised

[Jim Graham]

What is the need for this fix?  Is there a bug being fixed here other 
than you don't like the look of the code?

The reason I ask is that your fix requires a method call with a test for 
every graphics operation.  I'd prefer if we didn't add overhead unless 
it was necessary for correct function.

Also, the partially initialized state issue - while that technique can 
"in general" lead to issues, I don't think it is problematic here.  If 
that is the concern then we could target removing just that line.  Any 
references to the field from pipeline objects is safe since they won't 
be installed until a validate() is called which always sets the loops. 
The only suspect reference to loops would be the call from 
checkFontInfo() which might be invoked before the first validate() so it 
would need the protection against null, but almost no other piece of 
code you've modified can ever encounter a null loops field (or if it 
does then some validate() code forgot to set it)...

			...jim

Mario Torre wrote:
> Hello all!
> 
> While hacking on Cacio I've found that SunGraphics2D exposes a reference
> to "this" inside the constructor to another class, while initialising a
> field that contains the RederingLoops.
> 
> I filed a bug report and proposed a patch for review:
> 
> https://bugs.openjdk.java.net/show_bug.cgi?id=100068
> 
> The webrew is here:
> 
> http://cr.openjdk.java.net/~neugens/100068/webrev.01/
> 
> There is not much to say about the rationale for the bug/fix, just that
> the code looks a bit borked to me with those public references (there
> are others around, I think I should fix them all at some point), but the
> real problem is indeed exposing "this" in the constructor.
> 
> I hope I did the webrew correctly, as I had other patches around and no
> patch queue on this tree (yeah, yeah, I know...) so I had to do some
> manual tricks to make webrew happy, but the patch should be complete.
> 
> I tested with all the swing apps that come with OpenJDK, as well as
> those two nice things:
> 
> http://java.sun.com/products/java-media/2D/samples/index.html
> 
> I'll bug you every day if you make me wait too much about that review :)
> 
> Have fun,
> Mario