[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

[Laurent Bourgès]

Jim & Phil,

I updated my previous webrev to have MarlinRenderingEngine TL/CLQ more
close to the new proposed approach in AAShapePipe (and looks simpler):
http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.2/

Please give me your comments on my previous email.

Cheers,
Laurent


2016-02-06 0:21 GMT+01:00 Laurent Bourgès <bourges.laurent at gmail.com>:

> Jim & Phil,
>
> Here is an updated webrev according to jim's comments:
> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.1/
>
> Changes:
> - AAShapePipe rewritten to use a new ReentrantThreadLocal class
> (generalization) that use TL + CLQ (like Marlin) to use other (cached)
> TileState instances in case of reentrancy; such class may be useful for
> other use cases (replacing synchronized / static refs); it do not use
> WeakReference as TileState instances are very small (to be improved later ?)
> - Improved CrashPaintTest to check few pixel values to ensure rendering is
> correct
>
> PS: I noticed that AlphaPaintPipe has a constant int TILE_SIZE = 32 that
> seems useless as the method has a tilesize argument; maybe it should be
> cleaned up ?
>
> Regards,
> Laurent
>
> 2016-02-05 2:20 GMT+01:00 Jim Graham <james.graham at oracle.com>:
>
>> Ah, at least one error spotted here after my initial review...
>>
>> The abox values could be changed by the code in lines 157-160 so you
>> can't cache their values until after that.
>>
>> Also, as you pointed out in another thread in the grdev group, we should
>> probably do the same treatment for the TileState in case we have reentrance
>> for the AAShapePipe code...
>>
>>                 ...jim
>>
>>
>> On 2/4/2016 5:10 PM, Jim Graham wrote:
>>
>>> Hi Laurent,
>>>
>>> In AAShapePipe you load the values from abox[] into variables named
>>> xywh, but these are not xywh values, they are min/max values.  We
>>> typically use any of the following naming conventions for these types of
>>> values:
>>>
>>> - x0, y0, x1, y1
>>> - x1, y1, x2, y2
>>> - minX, minY, maxX, maxY
>>>
>>> Other than that naming inconsistency the changes look great...
>>>
>>>              ...jim
>>>
>>> On 2/4/2016 2:21 PM, Laurent Bourgès wrote:
>>>
>>>> Please review the webrev fixing SEGFAULT (P2) in the Marlin Renderer
>>>> when using thread-local storage with custom Paint (reentrance):
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8148886
>>>> webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.0/
>>>>
>>>> Changes:
>>>> - detect reentrance in MarlinRenderingEngine using flag
>>>> RendererContext.usedTL (true/false) and use another context from CLQ
>>>> - added few more details in array checks (XXXArrayCache)
>>>> - fixed AAShapePipe to support reentrancy: added defensive copy of int[]
>>>> abox in local variables + TODO to discuss
>>>> - updated Version to 0.7.3.2
>>>> - fixed copyright headers
>>>>
>>>> Best regards,
>>>> Laurent
>>>>
>>>
>
>
> --
> --
> Laurent Bourgès
>



-- 
-- 
Laurent Bourgès
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160208/30216559/attachment.html>