----- Original Message ----- > On 11/25/2015 06:53 PM, Andrew Hughes wrote: > > ----- 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. > > Here you go! > > https://jvanek.fedorapeople.org/oracle/jdk9/webrevs/resetInTryFinally/v2/ > > > > 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 > done > > > OpenJDK 9 for a new webrev. Once that is posted, I can commit > > done > > the change. > > > > TYVM! > > > I run all jtregs, and looked ok. > J. > >
Thanks. I cleaned up the test case formatting a little and pushed the change: http://hg.openjdk.java.net/jdk9/client/jdk/rev/284925b520f1 Once it's made it to the master jdk9 repository and had time to soak there, you probably want to suggest it for backport to 8u. 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
