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

Commit 43a4a8c7 authored by Christopher Tate's avatar Christopher Tate
Browse files

Fix redundant file backups 

We'd observed a bug in which an unchanged file was nevertheless
being redundantly transmitted for backup on every backup pass.
The underlying issue turns out to have been the FileBackupHelper
base implementation's logic for diffing the prior-state file
set against the current state, in the case when there had been
deletions of prior files.  In addition, there was also a
parallel bug in which file checksums were not calculated
properly in some cases, leading to at least one additional
redundant backup of the file in question.

Bug 18694053

Change-Id: Ie0dec06486b5fef4624561737019569c85d6b2a0
parent a2fa3d21

core/java/android/app/backup/BackupHelperDispatcher.java

+0 −1
Original line number Diff line number Diff line
@@ -50,7 +50,6 @@ public class BackupHelperDispatcher { 
        Header header = new Header();

        TreeMap<String,BackupHelper> helpers = (TreeMap<String,BackupHelper>)mHelpers.clone();

        FileDescriptor oldStateFD = null;

        FileDescriptor newStateFD = newState.getFileDescriptor();



        if (oldState != null) {

            oldStateFD = oldState.getFileDescriptor();

include/androidfw/BackupHelpers.h

+1 −1
Original line number Diff line number Diff line
@@ -152,7 +152,7 @@ private: 
    KeyedVector<String8,FileRec> m_files;

};



#define TEST_BACKUP_HELPERS 1

//#define TEST_BACKUP_HELPERS 1



#if TEST_BACKUP_HELPERS

int backup_helper_test_empty();

libs/androidfw/BackupHelpers.cpp

+41 −38
Original line number Diff line number Diff line
@@ -225,8 +225,6 @@ write_update_file(BackupDataWriter* dataStream, int fd, int mode, const String8& 
    file_metadata_v1 metadata;



    char* buf = (char*)malloc(bufsize);

    int crc = crc32(0L, Z_NULL, 0);





    fileSize = lseek(fd, 0, SEEK_END);

    lseek(fd, 0, SEEK_SET);
@@ -310,8 +308,12 @@ write_update_file(BackupDataWriter* dataStream, const String8& key, char const* 
}



static int

compute_crc32(int fd)

{

compute_crc32(const char* file, FileRec* out) {

    int fd = open(file, O_RDONLY);

    if (fd < 0) {

        return -1;

    }



    const int bufsize = 4*1024;

    int amt;
@@ -324,8 +326,11 @@ compute_crc32(int fd) 
        crc = crc32(crc, (Bytef*)buf, amt);

    }



    close(fd);

    free(buf);

    return crc;



    out->s.crc32 = crc;

    return NO_ERROR;

}



int
@@ -353,7 +358,8 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD 


        err = stat(file, &st);

        if (err != 0) {

            r.deleted = true;

            // not found => treat as deleted

            continue;

        } else {

            r.deleted = false;

            r.s.modTime_sec = st.st_mtime;
@@ -361,12 +367,17 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD 
            //r.s.modTime_nsec = st.st_mtime_nsec;

            r.s.mode = st.st_mode;

            r.s.size = st.st_size;

            // we compute the crc32 later down below, when we already have the file open.



            if (newSnapshot.indexOfKey(key) >= 0) {

                LOGP("back_up_files key already in use '%s'", key.string());

                return -1;

            }



            // compute the CRC

            if (compute_crc32(file, &r) != NO_ERROR) {

                ALOGW("Unable to open file %s", file);

                continue;

            }

        }

        newSnapshot.add(key, r);

    }
@@ -374,50 +385,42 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD 
    int n = 0;

    int N = oldSnapshot.size();

    int m = 0;

    int M = newSnapshot.size();



    while (n<N && m<fileCount) {

    while (n<N && m<M) {

        const String8& p = oldSnapshot.keyAt(n);

        const String8& q = newSnapshot.keyAt(m);

        FileRec& g = newSnapshot.editValueAt(m);

        int cmp = p.compare(q);

        if (g.deleted || cmp < 0) {

            // file removed

        if (cmp < 0) {

            // file present in oldSnapshot, but not present in newSnapshot

            LOGP("file removed: %s", p.string());

            g.deleted = true; // They didn't mention the file, but we noticed that it's gone.

            dataStream->WriteEntityHeader(p, -1);

            write_delete_file(dataStream, p);

            n++;

        }

        else if (cmp > 0) {

        } else if (cmp > 0) {

            // file added

            LOGP("file added: %s", g.file.string());

            LOGP("file added: %s crc=0x%08x", g.file.string(), g.s.crc32);

            write_update_file(dataStream, q, g.file.string());

            m++;

        }

        else {

            // both files exist, check them

            const FileState& f = oldSnapshot.valueAt(n);



            int fd = open(g.file.string(), O_RDONLY);

            if (fd < 0) {

                // We can't open the file.  Don't report it as a delete either.  Let the

                // server keep the old version.  Maybe they'll be able to deal with it

                // on restore.

                LOGP("Unable to open file %s - skipping", g.file.string());

        } else {

                g.s.crc32 = compute_crc32(fd);

            // same file exists in both old and new; check whether to update

            const FileState& f = oldSnapshot.valueAt(n);



            LOGP("%s", q.string());

                LOGP("  new: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",

                        f.modTime_sec, f.modTime_nsec, f.mode, f.size, f.crc32);

            LOGP("  old: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",

                    f.modTime_sec, f.modTime_nsec, f.mode, f.size, f.crc32);

            LOGP("  new: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",

                    g.s.modTime_sec, g.s.modTime_nsec, g.s.mode, g.s.size, g.s.crc32);

            if (f.modTime_sec != g.s.modTime_sec || f.modTime_nsec != g.s.modTime_nsec

                    || f.mode != g.s.mode || f.size != g.s.size || f.crc32 != g.s.crc32) {

                int fd = open(g.file.string(), O_RDONLY);

                if (fd < 0) {

                    ALOGE("Unable to read file for backup: %s", g.file.string());

                } else {

                    write_update_file(dataStream, fd, g.s.mode, p, g.file.string());

                }



                    close(fd);

                }

            }

            n++;

            m++;

        }
@@ -425,12 +428,12 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD 


    // these were deleted

    while (n<N) {

        dataStream->WriteEntityHeader(oldSnapshot.keyAt(n), -1);

        write_delete_file(dataStream, oldSnapshot.keyAt(n));

        n++;

    }



    // these were added

    while (m<fileCount) {

    while (m<M) {

        const String8& q = newSnapshot.keyAt(m);

        FileRec& g = newSnapshot.editValueAt(m);

        write_update_file(dataStream, q, g.file.string());