am 7d72ca0b: Avoid JsonWriter multi-write error

* commit '7d72ca0b20334aba077e3a01d7b12f6f34618076':
  Avoid JsonWriter multi-write error
main
Kurt Partridge 2013-05-10 19:21:16 -07:00 committed by Android Git Automerger
commit 03a3214645
4 changed files with 55 additions and 49 deletions

View File

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

View File

@ -23,6 +23,7 @@ import com.android.inputmethod.latin.Dictionary;
import com.android.inputmethod.latin.Suggest; import com.android.inputmethod.latin.Suggest;
import com.android.inputmethod.latin.define.ProductionFlag; import com.android.inputmethod.latin.define.ProductionFlag;
import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedList; import java.util.LinkedList;
@ -177,7 +178,7 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
return numWordsInLogUnitList == minNGramSize; return numWordsInLogUnitList == minNGramSize;
} }
public void shiftAndPublishAll() { public void shiftAndPublishAll() throws IOException {
final LinkedList<LogUnit> logUnits = getLogUnits(); final LinkedList<LogUnit> logUnits = getLogUnits();
while (!logUnits.isEmpty()) { while (!logUnits.isEmpty()) {
publishLogUnitsAtFrontOfBuffer(); publishLogUnitsAtFrontOfBuffer();
@ -186,10 +187,16 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
@Override @Override
protected final void onBufferFull() { 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 // TODO: Refactor this method to require fewer passes through the LogUnits. Should really
// require only one pass. // require only one pass.
ArrayList<LogUnit> logUnits = peekAtFirstNWords(N_GRAM_SIZE); 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 logUnits The list of logUnits to be published.
* @param canIncludePrivateData Whether the private data in the logUnits can be included in * @param canIncludePrivateData Whether the private data in the logUnits can be included in
* publication. * publication.
*
* @throws IOException if publication to the log file is not possible
*/ */
protected abstract void publish(final ArrayList<LogUnit> logUnits, protected abstract void publish(final ArrayList<LogUnit> logUnits,
final boolean canIncludePrivateData); final boolean canIncludePrivateData) throws IOException;
@Override @Override
protected int shiftOutWords(final int numWords) { protected int shiftOutWords(final int numWords) {

View File

@ -25,6 +25,7 @@ import com.android.inputmethod.latin.define.ProductionFlag;
import java.io.BufferedWriter; import java.io.BufferedWriter;
import java.io.File; import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
import java.io.OutputStreamWriter; import java.io.OutputStreamWriter;
@ -61,7 +62,11 @@ public class ResearchLog {
/* package */ final File mFile; /* package */ final File mFile;
private final Context mContext; 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 // 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 // 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 // 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. // could be caught, but this might suppress other errors.
private boolean mHasWrittenData = false; 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) { public ResearchLog(final File outputFile, final Context context) {
mExecutor = Executors.newSingleThreadScheduledExecutor(); mExecutor = Executors.newSingleThreadScheduledExecutor();
mFile = outputFile; mFile = outputFile;
@ -108,6 +93,7 @@ public class ResearchLog {
@Override @Override
public Object call() throws Exception { public Object call() throws Exception {
try { try {
if (mJsonWriter == null) return null;
// TODO: This is necessary to avoid an exception. Better would be to not even // 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 // open the JsonWriter if the file is not even opened unless there is valid data
// to write. // to write.
@ -119,9 +105,9 @@ public class ResearchLog {
mJsonWriter.flush(); mJsonWriter.flush();
mJsonWriter.close(); mJsonWriter.close();
if (DEBUG) { 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); Log.d(TAG, "error when closing ResearchLog:", e);
} finally { } finally {
// Marking the file as read-only signals that this log file is ready to be // Marking the file as read-only signals that this log file is ready to be
@ -162,6 +148,7 @@ public class ResearchLog {
@Override @Override
public Object call() throws Exception { public Object call() throws Exception {
try { try {
if (mJsonWriter == null) return null;
if (mHasWrittenData) { if (mHasWrittenData) {
// TODO: This is necessary to avoid an exception. Better would be to not // 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 // 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>() { private final Callable<Object> mFlushCallable = new Callable<Object>() {
@Override @Override
public Object call() throws Exception { public Object call() throws Exception {
mJsonWriter.flush(); if (mJsonWriter != null) mJsonWriter.flush();
return null; return null;
} }
}; };
@ -263,30 +250,29 @@ public class ResearchLog {
/** /**
* Return a JsonWriter for this ResearchLog. It is initialized the first time this method is * Return a JsonWriter for this ResearchLog. It is initialized the first time this method is
* called. The cached value is returned in future calls. * called. The cached value is returned in future calls.
*
* @throws IOException if opening the JsonWriter is not possible
*/ */
public JsonWriter getInitializedJsonWriterLocked() { public JsonWriter getInitializedJsonWriterLocked() throws IOException {
if (mJsonWriter != NULL_JSON_WRITER || mFile == null) return mJsonWriter; if (mJsonWriter != null) return mJsonWriter;
if (mFile == null) throw new FileNotFoundException();
try { try {
final JsonWriter jsonWriter = createJsonWriter(mContext, mFile); final JsonWriter jsonWriter = createJsonWriter(mContext, mFile);
if (jsonWriter != null) { if (jsonWriter == null) throw new IOException("Could not create JsonWriter");
jsonWriter.beginArray();
mJsonWriter = jsonWriter; jsonWriter.beginArray();
mHasWrittenData = true; mJsonWriter = jsonWriter;
} mHasWrittenData = true;
return mJsonWriter;
} catch (final IOException e) { } catch (final IOException e) {
Log.w(TAG, "Error in JsonWriter; disabling logging", e); if (DEBUG) {
try { Log.w(TAG, "Exception when creating JsonWriter", e);
mJsonWriter.close(); Log.w(TAG, "Closing JsonWriter");
} 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 (mJsonWriter != null) mJsonWriter.close();
mJsonWriter = null;
throw e;
} }
return mJsonWriter;
} }
/** /**

View File

@ -422,11 +422,19 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
// Commit mCurrentLogUnit before closing. // Commit mCurrentLogUnit before closing.
commitCurrentLogUnit(); commitCurrentLogUnit();
mMainLogBuffer.shiftAndPublishAll(); try {
mMainLogBuffer.shiftAndPublishAll();
} catch (final IOException e) {
Log.w(TAG, "IOException when publishing LogBuffer", e);
}
logStatistics(); logStatistics();
commitCurrentLogUnit(); commitCurrentLogUnit();
mMainLogBuffer.setIsStopping(); 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); mMainResearchLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);
mFeedbackLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS); mFeedbackLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);