----- Original Message ----- > On 11/18/2015 06:17 PM, Jiri Vanek wrote: > > On 11/12/2015 02:24 PM, Sergey Bylokhov wrote: > >> Hi, Jiri. > >> This is a valid point, did you file a new CR for this issue? > > ping? > > >> > > > > Hello! > > > > here it is: > > https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/ > > > > Patch: > > https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/webrev/ > > and reprodcuer > > https://jvanek.fedorapeople.org/oracle/jdk8/webrevs/resetInTryFinally/v1/MarkTryFinallyReproducer.java > > > > > > Reproducer can be easily turned to jtreg if wonted. > > > > The patch is for 8, but is valid also for 9, except the path to file. > > I will adapt it to 9 once you are ok with it (or you may on your own, > > depends on you) > > > > J. > > > > > >> On 09.11.15 15:45, Alexander Scherbatiy wrote: > >>> On 11/7/2015 11:38 AM, Jiri Vanek wrote: > >>>> Hello! > >>>> > >>>> Looking to imageIO.java (if this is bad thread, please redirect me!) > >>> > >>> 2d-dev alias should be also the right place to ask image related > >>> questions in AWT. > >>> > >>> Thanks, > >>> Alexandr. > >>> > >>>> when reading images > >>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/javax/imageio/ImageIO.java#l543 > >>>> > >>>> > >>>> > >>>> and ending at: > >>>> > >>>> // Perform mark/reset as a defensive measure > >>>> // even though plug-ins are supposed to take > >>>> // care of it. > >>>> boolean canDecode = false; > >>>> if (stream != null) { > >>>> stream.mark(); > >>>> } > >>>> canDecode = spi.canDecodeInput(input); > >>>> if (stream != null) { > >>>> stream.reset(); > >>>> } > >>>> return canDecode; > >>>> > >>>> I'm wondering, why stream.reset(); is not in finaly block: > >>>> > >>>> // Perform mark/reset as a defensive measure > >>>> // even though plug-ins are supposed to take > >>>> // care of it. > >>>> boolean canDecode = false; > >>>> if (stream != null) { > >>>> stream.mark(); > >>>> } > >>>> try{ > >>>> canDecode = spi.canDecodeInput(input); > >>>> } finally { > >>>> if (stream != null) { > >>>> stream.reset(); > >>>> } > >>>> } > >>>> return canDecode; > >>>> > >>>> > >>>> Eg png and bmp decoders can are throwing IIOException when header is > >>>> corrutped. That pretty definitely sure killer of stream mark stacks. > >>>> You yourselves write : //Perform mark/reset as a defensive measure > >>>> even though plug-ins are supposed to take care of it. > >>>> So if densive, then try/finaly please. > >>>> > >>>> I did not check if this changed in 9 but if not....Please. This is > >>>> makig work on custom image plugins, for image formats which just wraps > >>>> another bmp/png and are expecting corrupted headers preatty hards. > >>>> > >>>> > >>>> J. > >>> > >> > >> > > > >
I've filed https://bugs.openjdk.java.net/browse/JDK-8144071 for this. The change looks good to me. I can confirm the reproducer fails on OpenJDK 6, 7 and 8: Exception in thread "main" java.lang.RuntimeException: exeption from call to canDecodeInput in ImageIO corrupted stack in ImageInputStream at MarkTryFinallyReproducer.main(MarkTryFinallyReproducer.java:317) I suggest the test case is converted to jtreg and the patch to OpenJDK 9 for a new webrev. Once that is posted, I can commit the change. Thanks, -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: ed25519/35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222 PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
