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

Commit e491d142 authored by Christopher Tate's avatar Christopher Tate
Browse files

Fix BlobBackupHelper state tracking 

The disk-friendly use of BufferedInputStream in state parsing was
actually breaking the state management system when multiple helpers
are involved in backup.  The BufferedInputStream would blithely
pre-fetch 8K from the state file -- but that would consume some
state data that the dispatcher and subsequent helpers needed to see
in order to properly evaluate prior state themselves.  The contract
here is that when using the helper mechanism, state-data consumption
must *only* consume as much data as the helper originally stored
into the file.

Switching to a direct-read stream instead fixes the problem by
avoiding read overrun.  The rest of the changes are expanded
debugging output that made diagnosis possible.

Bug 25473872

Change-Id: I91de38dfec96f589700479f41ac26f144ba4463c
parent 4f160735

core/java/android/app/backup/BlobBackupHelper.java

+31 −9
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import android.os.ParcelFileDescriptor; 
import android.util.ArrayMap;

import android.util.Log;



import java.io.BufferedInputStream;

import java.io.ByteArrayInputStream;

import java.io.ByteArrayOutputStream;

import java.io.DataInputStream;
@@ -42,7 +41,7 @@ import java.util.zip.InflaterInputStream; 
 */

public abstract class BlobBackupHelper implements BackupHelper {

    private static final String TAG = "BlobBackupHelper";

    private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);

    private static final boolean DEBUG = false;



    private final int mCurrentBlobVersion;

    private final String[] mKeys;
@@ -92,16 +91,21 @@ public abstract class BlobBackupHelper implements BackupHelper { 
        final ArrayMap<String, Long> state = new ArrayMap<String, Long>();



        FileInputStream fis = new FileInputStream(oldStateFd.getFileDescriptor());

        BufferedInputStream bis = new BufferedInputStream(fis);

        DataInputStream in = new DataInputStream(bis);

        DataInputStream in = new DataInputStream(fis);



        try {

            int version = in.readInt();

            if (version <= mCurrentBlobVersion) {

                final int numKeys = in.readInt();

                if (DEBUG) {

                    Log.i(TAG, "  " + numKeys + " keys in state record");

                }

                for (int i = 0; i < numKeys; i++) {

                    String key = in.readUTF();

                    long checksum = in.readLong();

                    if (DEBUG) {

                        Log.i(TAG, "  key '" + key + "' checksum is " + checksum);

                    }

                    state.put(key, checksum);

                }

            } else {
@@ -110,6 +114,9 @@ public abstract class BlobBackupHelper implements BackupHelper { 
        } catch (EOFException e) {

            // Empty file is expected on first backup,  so carry on. If the state

            // is truncated we just treat it the same way.

            if (DEBUG) {

                Log.i(TAG, "Hit EOF reading prior state");

            }

            state.clear();

        } catch (Exception e) {

            Log.e(TAG, "Error examining prior backup state " + e.getMessage());
@@ -136,8 +143,13 @@ public abstract class BlobBackupHelper implements BackupHelper { 
            final int N = (state != null) ? state.size() : 0;

            out.writeInt(N);

            for (int i = 0; i < N; i++) {

                out.writeUTF(state.keyAt(i));

                out.writeLong(state.valueAt(i).longValue());

                final String key = state.keyAt(i);

                final long checksum = state.valueAt(i).longValue();

                if (DEBUG) {

                    Log.i(TAG, "  writing key " + key + " checksum = " + checksum);

                }

                out.writeUTF(key);

                out.writeLong(checksum);

            }

        } catch (IOException e) {

            Log.e(TAG, "Unable to write updated state", e);
@@ -226,6 +238,9 @@ public abstract class BlobBackupHelper implements BackupHelper { 
    @Override

    public void performBackup(ParcelFileDescriptor oldStateFd, BackupDataOutput data,

            ParcelFileDescriptor newStateFd) {

        if (DEBUG) {

            Log.i(TAG, "Performing backup for " + this.getClass().getName());

        }



        final ArrayMap<String, Long> oldState = readOldState(oldStateFd);

        final ArrayMap<String, Long> newState = new ArrayMap<String, Long>();
@@ -234,12 +249,16 @@ public abstract class BlobBackupHelper implements BackupHelper { 
            for (String key : mKeys) {

                final byte[] payload = deflate(getBackupPayload(key));

                final long checksum = checksum(payload);

                if (DEBUG) {

                    Log.i(TAG, "Key " + key + " backup checksum is " + checksum);

                }

                newState.put(key, checksum);



                Long oldChecksum = oldState.get(key);

                if (oldChecksum == null || checksum != oldChecksum) {

                if (oldChecksum == null || checksum != oldChecksum.longValue()) {

                    if (DEBUG) {

                        Log.i(TAG, "State has changed for key " + key + ", writing");

                        Log.i(TAG, "Checksum has changed from " + oldChecksum + " to " + checksum

                                + " for key " + key + ", writing");

                    }

                    if (payload != null) {

                        data.writeEntityHeader(key, payload.length);
@@ -258,7 +277,7 @@ public abstract class BlobBackupHelper implements BackupHelper { 
            Log.w(TAG,  "Unable to record notification state: " + e.getMessage());

            newState.clear();

        } finally {

            // Always recommit the state even if nothing changed

            // Always rewrite the state even if nothing changed

            writeBackupState(newState, newStateFd);

        }

    }
@@ -291,6 +310,9 @@ public abstract class BlobBackupHelper implements BackupHelper { 
    @Override

    public void writeNewStateDescription(ParcelFileDescriptor newState) {

        // Just ensure that we do a full backup the first time after a restore

        if (DEBUG) {

            Log.i(TAG, "Writing state description after restore");

        }

        writeBackupState(null, newState);

    }

}