On Feb 5, 2015, at 9:50 PM, Xueming Shen <xueming.s...@oracle.com> wrote:
> On 02/05/2015 12:47 PM, Paul Sandoz wrote: >> On Feb 5, 2015, at 8:00 PM, Xueming Shen<xueming.s...@oracle.com> wrote: >> >>> Hi, >>> >>> Please help review the fix for #8030179 >>> >>> issue: https://bugs.openjdk.java.net/browse/JDK-8030179 >>> webrev: http://cr.openjdk.java.net/~sherman/8030179/webrev >>> >>> This is the regression bug introduced in jdk7 when trying to optimize the >>> single >>> byte encoding loop, in which the "optimization" code inappropriately >>> updates the >>> "sl" (source limit) value and triggers misbehavior of the sgp parser (in >>> which it >>> mistakenly returns "underflow" when it sees a high surrogate but can't see >>> the next >>> low surrogate, because the "sl" is changed...) >>> >> The fix looks good, but i would like to suggest some changes to the test. >> >> We can use TestNG to avoid pulling in a bunch of infrastructure methods. >> >> The test cases can be produced by a data provider and we can do a cross >> product with char sets and character input (for example i have managed to >> induced stack overflows by just passing sequences of high surrogates). >> >> Using the data provider allows the test infrastrcture to report/crunch those >> tests individually rather than being reported via System.out. >> >> I can send a patch to the test if that helps. > > That will be appreciated :-) > See below. On my Mac this runs 504 tests, 4 to 6 of which fail (i dunno where the non-determinism comes from). In the jtreg reported output you will see stuff like this: test StreamEncoderOut.test(x-windows-iso2022jp, HIGH_LOW : '\ud834\udd1e"): success Unfortunately because a StackOverflowException is printed out jtreg truncates the middle part of the output (and the system property jtreg says one can use to increase that output does not appear to work) so it's tricky to work out what actually failed without explicitly catching the SOE and re-throwing as an assertion error. (We should have a better way of printing out the stack trace of an SOE.) So you could remove the Input.HIGH value in the test, commit, and log a another bug for the SOE, then add it back for the fix. Paul. /* @test @bug 8030179 @summary test if the charset encoder deails with surrogate correctly * @run testng/othervm -esa StreamEncoderOut */ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.nio.charset.Charset; import java.util.stream.Stream; import static java.util.stream.Collectors.joining; @Test public class StreamEncoderOut { enum Input { HIGH("\ud834"), LOW("\udd1e"), HIGH_LOW("\ud834\udd1e"); final String value; Input(String value) { this.value = value; } @Override public String toString() { return name() + " : \'" + value + "\""; } } @DataProvider(name = "CharsetAndString") // [Charset, Input] public static Object[][] makeStreamTestData() { // Cross product of supported charsets and inputs return Charset.availableCharsets().values().stream(). filter(Charset::canEncode). flatMap(c -> Stream.of(Input.values()).map(i -> new Object[]{c, i})). toArray(Object[][]::new); } private static String generate(String s, int n) { return Stream.generate(() -> s).limit(n).collect(joining()); } static final OutputStream DEV_NULL = new OutputStream() { @Override public void write(byte b[], int off, int len) throws IOException {} @Override public void write(int b) throws IOException {} }; @Test(dataProvider = "CharsetAndString") public void test(Charset cs, Input input) throws IOException { OutputStreamWriter w = new OutputStreamWriter(DEV_NULL, cs); String t = generate(input.value, 8193); for (int i = 0; i < 10; i++) { w.append(t); } } }
