ok. Thanks Jim .
Please review the modified webrev
http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.02/
Regards
Prasanta
On 12/4/2015 6:45 AM, Jim Graham wrote:
I think it makes sense to catch it at a higher level, but also to
throw some type of exception from the X11 code as you do now because
regardless of our higher level policy, the X11 implementation function
can never succeed there...
So, my preference would be to keep the existing pieces of this fix
that you already have and to add an explicit check and throw IAE to
SunVolatileImage for completeness and guaranteed consistency...
...jim
On 12/2/15 11:18 PM, prasanta sadhukhan wrote:
Hi Jim,
On 12/3/2015 12:38 AM, Jim Graham wrote:
Hi Prasanta,
(On a practical note, in the HTML version of your message, the text
said "webrev.01", but the link in the href pointed to "webrev.00" so I
sat there wondering why the changes you noted weren't there until I
realized that I was still looking at webrev.00 and had to manually
enter webrev.01 in the browser to see the new code...)
Have you run your new test on all platforms to make sure that it
succeeds (by throwing IAE) on all currently supported/tested platorms?
IAE was already been thrown for windows and mac. It was not working for
linux only.
It seems, from the comment, that one issue is that X11 has a special
need in that if we make it through to the native code with 0,0
arguments and attempt to create a pixmap of 0,0 then we get an X11
error so I'm OK with the native code having its own check for
protection against the X11 error. But, for consistency, shouldn't the
0,0 be detected and and IAE thrown at a much higher level shared by
all platforms so that no platform can accidentally allow 0,0?
Otherwise we have to make sure that each and every current platform
and each and every future platform port contains these checks to
satisfy the new behavior expectation.
Apparently, somewhere above the native method there is a check that
converts OOME to IAE - is that in shared code or in the X11-specific
code?
It is not a direct conversion from OOME to IAE. Basically, the Java will
try to create a accelerated surface and if it cannot, it creates a
backup BufferedImage via createCompatibleImage() which calls
createCompatibleWritableRaster() and that function has a check for 0
width,height which throws IAE.
Code wise flow:
VolatileSurfaceManager.initialize() will check for accelerated surface
via initAcceleratedSurface() which goes to different pipeline for
different platforms.
In windows, D3DVolatileSurfaceManager.java#initAcceleratedSurface() will
call D3DSurfaceData.createData() which calls initSurface() which
initializes surface by calling "native" initSurfaceNow() which returns
false for 0 width.height. D3DSurfaceData throws InvalidPipeException to
D3DVolatileSurfaceManager#initAcceleratedSurface() which marks
accelerated surface as null in that case so Java will fall back to
BufferedImage as explained above.
In mac, CGLVolatileSurfaceManager.jaav#initAcceleratedSurface() will get
OOM from CGlSurfaceData.createData() which calls native initFBObject()
which returns false
In linux, native was not throwing any issue even though it does not
utilise 0 width,height so Java still assumes it is working with
accelerated surface so will not try to create backup BufferedImage
surface so cannot throw IAE.
My fix will let native throw OOM to
XRVolatileSurfaceManager.java#initAcceleratedSurface() so that it knows
it fails to create accelerated surface and has to fall back to
BufferedImage surface and then the IAE will be thrown from
createCompatibleWritableRaster()
If you think it's ok, we can catch 0 width,height in SunVolatileImage
constructor before it calls VolatileSurfacemaanger. initialize() so that
it gets trapped at higher level? Please advise.
Regards
Prasanta
...jim
On 11/30/15 9:58 PM, prasanta sadhukhan wrote:
Modified the testcase to "fail" if IAE is not thrown
http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.01
Regards
Prasanta
On 11/30/2015 2:13 PM, prasanta sadhukhan wrote:
Hi All,
Please review a fix for jdk9
Bug: https://bugs.openjdk.java.net/browse/JDK-8140530
webrev: http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.00/
The issue was creating a volatileImage with 0 width, height does not
result in IllegalArgumentException.
But, when we try to create a non-volatile Image via
GraphicsConfiguration.createCompatibleImage(0,0) or a
BufferedImage(0,0,imagetype) it results in IAE.
So, to maintain consistency across all image w.r.t 0 width,height,
createVolatileImage() should also throw IAE.
In windows, creating a volatileImage with 0 width,height resulted in
IAE but in linux it does not.
This is because XCreatePixmap() generate BadValue unless width,height
is nonzero but the error handler does not catch it.
https://tronche.com/gui/x/xlib/pixmap-and-cursor/XCreatePixmap.html
[The width and height arguments must be nonzero, or a *BadValue*
error
results.]
I have added a check to prevent zero width,height to be used for
XCreatePixmap() and also throw OOME so to ask Java to throw IAE.
Regards
Prasanta
