Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7d72ca0b authored by Kurt Partridge's avatar Kurt Partridge
Browse files

Avoid JsonWriter multi-write error

JsonWriter requires that its clients pass it only a single top-level object.
The existing implementation tries to make code cleaner by having mJsonWriter
never be null, and instead use a global static "NULL_JSON_WRITER" that just
discards data.  But because JsonWriter complains if more than one top-level
object is passed, making this a global object does not work.  This change
instead copes with mJsonWriter being null.

Change-Id: Ia37ccfc8646e91f11a64713dd92d2846eb86ac54
parent 260df841
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import com.android.inputmethod.latin.SuggestedWords;
import com.android.inputmethod.latin.SuggestedWords.SuggestedWordInfo;
import com.android.inputmethod.latin.define.ProductionFlag;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -135,9 +136,11 @@ public class LogUnit {
     * @param researchLog where to publish the contents of this {@code LogUnit}
     * @param canIncludePrivateData whether the private data in this {@code LogUnit} should be
     * included
     *
     * @throws IOException if publication to the log file is not possible
     */
    public synchronized void publishTo(final ResearchLog researchLog,
            final boolean canIncludePrivateData) {
            final boolean canIncludePrivateData) throws IOException {
        // Write out any logStatement that passes the privacy filter.
        final int size = mLogStatementList.size();
        if (size != 0) {
+13 −4
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import com.android.inputmethod.latin.Dictionary;
import com.android.inputmethod.latin.Suggest;
import com.android.inputmethod.latin.define.ProductionFlag;

import java.io.IOException;
import java.util.ArrayList;
import java.util.LinkedList;

@@ -177,7 +178,7 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
        return numWordsInLogUnitList == minNGramSize;
    }

    public void shiftAndPublishAll() {
    public void shiftAndPublishAll() throws IOException {
        final LinkedList<LogUnit> logUnits = getLogUnits();
        while (!logUnits.isEmpty()) {
            publishLogUnitsAtFrontOfBuffer();
@@ -186,10 +187,16 @@ public abstract class MainLogBuffer extends FixedLogBuffer {

    @Override
    protected final void onBufferFull() {
        try {
            publishLogUnitsAtFrontOfBuffer();
        } catch (final IOException e) {
            if (DEBUG) {
                Log.w(TAG, "IOException when publishing front of LogBuffer", e);
            }
        }
    }

    protected final void publishLogUnitsAtFrontOfBuffer() {
    protected final void publishLogUnitsAtFrontOfBuffer() throws IOException {
        // TODO: Refactor this method to require fewer passes through the LogUnits.  Should really
        // require only one pass.
        ArrayList<LogUnit> logUnits = peekAtFirstNWords(N_GRAM_SIZE);
@@ -224,9 +231,11 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
     * @param logUnits The list of logUnits to be published.
     * @param canIncludePrivateData Whether the private data in the logUnits can be included in
     * publication.
     *
     * @throws IOException if publication to the log file is not possible
     */
    protected abstract void publish(final ArrayList<LogUnit> logUnits,
            final boolean canIncludePrivateData);
            final boolean canIncludePrivateData) throws IOException;

    @Override
    protected int shiftOutWords(final int numWords) {
+28 −42
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import com.android.inputmethod.latin.define.ProductionFlag;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
@@ -61,7 +62,11 @@ public class ResearchLog {
    /* package */ final File mFile;
    private final Context mContext;

    private JsonWriter mJsonWriter = NULL_JSON_WRITER;
    // Earlier implementations used a dummy JsonWriter that just swallowed what it was given, but
    // this was tricky to do well, because JsonWriter throws an exception if it is passed more than
    // one top-level object.
    private JsonWriter mJsonWriter = null;

    // true if at least one byte of data has been written out to the log file.  This must be
    // remembered because JsonWriter requires that calls matching calls to beginObject and
    // endObject, as well as beginArray and endArray, and the file is opened lazily, only when
@@ -69,26 +74,6 @@ public class ResearchLog {
    // could be caught, but this might suppress other errors.
    private boolean mHasWrittenData = false;

    private static final JsonWriter NULL_JSON_WRITER = new JsonWriter(
            new OutputStreamWriter(new NullOutputStream()));
    private static class NullOutputStream extends OutputStream {
        /** {@inheritDoc} */
        @Override
        public void write(byte[] buffer, int offset, int count) {
            // nop
        }

        /** {@inheritDoc} */
        @Override
        public void write(byte[] buffer) {
            // nop
        }

        @Override
        public void write(int oneByte) {
        }
    }

    public ResearchLog(final File outputFile, final Context context) {
        mExecutor = Executors.newSingleThreadScheduledExecutor();
        mFile = outputFile;
@@ -108,6 +93,7 @@ public class ResearchLog {
            @Override
            public Object call() throws Exception {
                try {
                    if (mJsonWriter == null) return null;
                    // TODO: This is necessary to avoid an exception.  Better would be to not even
                    // open the JsonWriter if the file is not even opened unless there is valid data
                    // to write.
@@ -119,9 +105,9 @@ public class ResearchLog {
                    mJsonWriter.flush();
                    mJsonWriter.close();
                    if (DEBUG) {
                        Log.d(TAG, "wrote log to " + mFile);
                        Log.d(TAG, "closed " + mFile);
                    }
                } catch (Exception e) {
                } catch (final Exception e) {
                    Log.d(TAG, "error when closing ResearchLog:", e);
                } finally {
                    // Marking the file as read-only signals that this log file is ready to be
@@ -162,6 +148,7 @@ public class ResearchLog {
            @Override
            public Object call() throws Exception {
                try {
                    if (mJsonWriter == null) return null;
                    if (mHasWrittenData) {
                        // TODO: This is necessary to avoid an exception.  Better would be to not
                        // even open the JsonWriter if the file is not even opened unless there is
@@ -217,7 +204,7 @@ public class ResearchLog {
    private final Callable<Object> mFlushCallable = new Callable<Object>() {
        @Override
        public Object call() throws Exception {
            mJsonWriter.flush();
            if (mJsonWriter != null) mJsonWriter.flush();
            return null;
        }
    };
@@ -263,30 +250,29 @@ public class ResearchLog {
    /**
     * Return a JsonWriter for this ResearchLog.  It is initialized the first time this method is
     * called.  The cached value is returned in future calls.
     *
     * @throws IOException if opening the JsonWriter is not possible
     */
    public JsonWriter getInitializedJsonWriterLocked() {
        if (mJsonWriter != NULL_JSON_WRITER || mFile == null) return mJsonWriter;
    public JsonWriter getInitializedJsonWriterLocked() throws IOException {
        if (mJsonWriter != null) return mJsonWriter;
        if (mFile == null) throw new FileNotFoundException();
        try {
            final JsonWriter jsonWriter = createJsonWriter(mContext, mFile);
            if (jsonWriter != null) {
            if (jsonWriter == null) throw new IOException("Could not create JsonWriter");

            jsonWriter.beginArray();
            mJsonWriter = jsonWriter;
            mHasWrittenData = true;
            }
            return mJsonWriter;
        } catch (final IOException e) {
            Log.w(TAG, "Error in JsonWriter; disabling logging", e);
            try {
                mJsonWriter.close();
            } catch (final IllegalStateException e1) {
                // Assume that this is just the json not being terminated properly.
                // Ignore
            } catch (final IOException e1) {
                Log.w(TAG, "Error in closing JsonWriter; disabling logging", e1);
            } finally {
                mJsonWriter = NULL_JSON_WRITER;
            if (DEBUG) {
                Log.w(TAG, "Exception when creating JsonWriter", e);
                Log.w(TAG, "Closing JsonWriter");
            }
            if (mJsonWriter != null) mJsonWriter.close();
            mJsonWriter = null;
            throw e;
        }
        return mJsonWriter;
    }

    /**
+10 −2
Original line number Diff line number Diff line
@@ -422,11 +422,19 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
        // Commit mCurrentLogUnit before closing.
        commitCurrentLogUnit();

        try {
            mMainLogBuffer.shiftAndPublishAll();
        } catch (final IOException e) {
            Log.w(TAG, "IOException when publishing LogBuffer", e);
        }
        logStatistics();
        commitCurrentLogUnit();
        mMainLogBuffer.setIsStopping();
        try {
            mMainLogBuffer.shiftAndPublishAll();
        } catch (final IOException e) {
            Log.w(TAG, "IOException when publishing LogBuffer", e);
        }
        mMainResearchLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);
        mFeedbackLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);