You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Deven You <de...@gmail.com> on 2010/07/21 15:25:23 UTC

Re: Fwd: [jira] Commented: (HARMONY-6590) [classlib][luni]A issue about CharsetEncoder.flush() in the OutputStreamWriter.close()

2010/7/21 Tim Ellison <t....@gmail.com>

> On 20/Jul/2010 04:16, Deven You wrote:
> > Hi Tim,
> > I have write a test case which simulate the behavior of
> OutputStreamWriter
> > and fail on RI but pass on harmony trunk.
>
> yes, and the discussion of how CharsetEncoder behaves is the first thing
> -- but then we should look at how the OutputStreamWriter uses the
> encoder.  Its not clear to me yet that there is a problem to fix in
> OutputStreamWriter.
>
Suppose our CharsetEncoder.flush() follows the spec and RI5, below test case
should fail:
        BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(new
ByteArrayOutputStream()));
            bw.close();
bw.close() invoke the OutputStreamWriter.close which invokes the
encoder.flush().

In this test case, the invocation sequence of harmony OutputStreamWriter is
OutputStreamWriter(OutputStrem) -> OutputStreamWriter.close(), in this
sequence there is no other encode operations are called except
encoder.flush(). With such situation, if our encoder.flush() follows the
spec and RI5, the flush() should throw IlegalStateException.

So our OutputStreamWriter pass this test not because its logic is right but
because our CharsetEncoder also disobey the spec and RI5. they by chance
together led to this result.

If we fix the CharsetEncoder.flush() to folllow the spec and RI5, the above
test case will fail.

And I have created JIRA 6594 to solve the CharsetEncoder.flush() issue,
after this patch has been applied, the above test case will fail. And then
need this JIRA 6590 to fix the above test case.

I'm not sure if this information is enough for you. Sorry for confusing you
so much :). Any problems let me know. Thanks a lot!

> Regards,
> Tim
>
>
> > import java.nio.ByteBuffer;
> > import java.nio.charset.Charset;
> > import java.nio.charset.CharsetEncoder;
> > import java.nio.charset.CodingErrorAction;
> > import java.security.AccessController;
> >
> > import org.apache.harmony.luni.util.PriviAction;
> >
> > public class TestCharsetEncoderFlush {
> >  private static ByteBuffer bytes = ByteBuffer.allocate(8192);
> >     private static CharsetEncoder encoder;
> >
> > public static void main(String[] args) {
> > String encoding = AccessController
> > .doPrivileged(new PriviAction<String>(
> > "file.encoding", "ISO8859_1")); //$NON-NLS-1$ //$NON-NLS-2$
> > encoder = Charset.forName(encoding).newEncoder();
> > encoder.onMalformedInput(CodingErrorAction.REPLACE);
> > encoder.onUnmappableCharacter(CodingErrorAction.REPLACE);
> >  encoder.flush(bytes);
> >  System.out.println("should not reach here");
> >
> > }
> >
> > }
> >
> > RI output:
> > Exception in thread "main" java.lang.IllegalStateException: Current state
> =
> > RESET, new state = FLUSHED
> > at
> >
> java.nio.charset.CharsetEncoder.throwIllegalStateException(CharsetEncoder.java:951)
> > at java.nio.charset.CharsetEncoder.flush(CharsetEncoder.java:640)
> > at
> >
> ydw.arena7.luni.test.TestCharsetEncoderFlush.main(TestCharsetEncoderFlush.java:27)
> > Harmony trunk output:
> > should not reach here
> >
> > It proves our CharsetEncoder indeed does not follow the spec and RI. I
> will
> > create a jira with this test case and figure out how to solve this
> problem.
> >>From the spec, seems we need track every step of  CharsetEncoder
> invocation,
> > but it looks silly, dose anyone have better suggestion for this problem.
> > Thanks a lot!
> >
> > ---------- Forwarded message ----------
> > From: Tim Ellison (JIRA) <ji...@apache.org>
> > Date: 2010/7/19
> > Subject: [jira] Commented: (HARMONY-6590) [classlib][luni]A issue about
> > CharsetEncoder.flush() in the OutputStreamWriter.close()
> > To: devyoudw@gmail.com
> >
> >
> >
> >    [
> >
> https://issues.apache.org/jira/browse/HARMONY-6590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12889841#action_12889841
> ]
> >
> > Tim Ellison commented on HARMONY-6590:
> > --------------------------------------
> >
> > Interesting that this code does not fail on the RI, note that the encoder
> is
> > in the 'wrong' state.
> >
> >        OutputStream os = new ByteArrayOutputStream();
> >        CharsetEncoder cse = Charset.defaultCharset().newEncoder();
> >        cse.encode(CharBuffer.wrap("Simple"), ByteBuffer.allocate(42),
> > false);
> >        OutputStreamWriter bw = new OutputStreamWriter(os, cse);
> >        bw.flush();
> >        bw.close();
> >
> > Maybe the flush() doesn't flush the encoder.
> >
> >> [classlib][luni]A issue about CharsetEncoder.flush() in the
> > OutputStreamWriter.close()
> >
> --------------------------------------------------------------------------------------
> >>                 Key: HARMONY-6590
> >>                 URL: https://issues.apache.org/jira/browse/HARMONY-6590
> >>             Project: Harmony
> >>          Issue Type: Bug
> >>          Components: Classlib
> >>    Affects Versions: 5.0M14
> >>            Reporter: deven you
> >>            Assignee: Tim Ellison
> >>         Attachments: HARMONY-6590.diff
> >>
> >>   Original Estimate: 96h
> >>  Remaining Estimate: 96h
> >>
> >> Today I read through the OutputStreamWrtier.close() code below:
> >>     public void close() throws IOException {
> >>         synchronized (lock) {
> >>             if (encoder != null) {
> >>                 encoder.flush(bytes);
> >>                 flush();
> >>                 out.flush();
> >>                 out.close();
> >>                 encoder = null;
> >>                 bytes = null;
> >>             }
> >>         }
> >>     }
> >> I remember the java spec says for the CharsetEncoder.flush():
> > IllegalStateException - If the previous step of the current encoding
> > operation was an invocation neither of the reset method nor of the
> > three-argument encode method with a value of true for the endOfInput
> > parameter.
> >> Obviously OutputStreamWrtier.close() does not check this prerequisite
> > before invoking the encoder.flush(bytes). So I write a test case[1] to
> check
> > this issue but it passed, I think it is because our
> CharsetEncoder.flush()
> > does not follow the spec.
> >> Though I think our OutputStreamWrtier.close() should modify to follow
> the
> > spec. I have put the patch[1] on this jira.
> >> And I will also look into the CharsetEncoder.flush() to investigate this
> > problem.
> >> [1] see the attached patch
> >
> > --
> > This message is automatically generated by JIRA.
> > -
> > You can reply to this email to add a comment to the issue online.
> >
>