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
diff --git a/java/src/com/android/inputmethod/research/LogUnit.java b/java/src/com/android/inputmethod/research/LogUnit.java
index 4d60bda..cf1388f 100644
--- a/java/src/com/android/inputmethod/research/LogUnit.java
+++ b/java/src/com/android/inputmethod/research/LogUnit.java
@@ -25,6 +25,7 @@
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 @@
* @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) {
diff --git a/java/src/com/android/inputmethod/research/MainLogBuffer.java b/java/src/com/android/inputmethod/research/MainLogBuffer.java
index 9bdedbf..9aa3499 100644
--- a/java/src/com/android/inputmethod/research/MainLogBuffer.java
+++ b/java/src/com/android/inputmethod/research/MainLogBuffer.java
@@ -23,6 +23,7 @@
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 @@
return numWordsInLogUnitList == minNGramSize;
}
- public void shiftAndPublishAll() {
+ public void shiftAndPublishAll() throws IOException {
final LinkedList<LogUnit> logUnits = getLogUnits();
while (!logUnits.isEmpty()) {
publishLogUnitsAtFrontOfBuffer();
@@ -186,10 +187,16 @@
@Override
protected final void onBufferFull() {
- publishLogUnitsAtFrontOfBuffer();
+ 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 @@
* @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) {
diff --git a/java/src/com/android/inputmethod/research/ResearchLog.java b/java/src/com/android/inputmethod/research/ResearchLog.java
index 18bf7ba..3e82139 100644
--- a/java/src/com/android/inputmethod/research/ResearchLog.java
+++ b/java/src/com/android/inputmethod/research/ResearchLog.java
@@ -25,6 +25,7 @@
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 @@
/* 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 @@
// 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 @@
@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 @@
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 @@
@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 @@
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 @@
/**
* 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) {
- jsonWriter.beginArray();
- mJsonWriter = jsonWriter;
- mHasWrittenData = true;
- }
+ 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;
}
/**
diff --git a/java/src/com/android/inputmethod/research/ResearchLogger.java b/java/src/com/android/inputmethod/research/ResearchLogger.java
index 4e839f9..b3c0faf 100644
--- a/java/src/com/android/inputmethod/research/ResearchLogger.java
+++ b/java/src/com/android/inputmethod/research/ResearchLogger.java
@@ -422,11 +422,19 @@
// Commit mCurrentLogUnit before closing.
commitCurrentLogUnit();
- mMainLogBuffer.shiftAndPublishAll();
+ try {
+ mMainLogBuffer.shiftAndPublishAll();
+ } catch (final IOException e) {
+ Log.w(TAG, "IOException when publishing LogBuffer", e);
+ }
logStatistics();
commitCurrentLogUnit();
mMainLogBuffer.setIsStopping();
- mMainLogBuffer.shiftAndPublishAll();
+ 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);
