Thanks Paul!
Webrev has been updated to include your test, and to fix that
stackoverflowexception, triggered
by the recursive implementation of StreamEncoder.flushLeftoverChar. The method
really does
not need to be recursive (an alternative is to push the "leftover" from the
input buffer back to
the buffer, but it appears the proposed change is simple)
http://cr.openjdk.java.net/~sherman/8030179/webrev/
-Sherman
On 02/06/2015 02:03 AM, Paul Sandoz wrote:
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);
}
}
}