fast-import: Avoid infinite loop after reset

Johannes Sixt noticed that a 'reset' command applied to a branch that
is already active in the branch LRU cache can cause fast-import to
relink the same branch into the LRU cache twice.  This will cause
the LRU cache to contain a cycle, making unload_one_branch run in an
infinite loop as it tries to select the oldest branch for eviction.

I have trivially fixed the problem by adding an active bit to
each branch object; this bit indicates if the branch is already
in the LRU and allows us to avoid trying to add it a second time.
Converting the pack_id field into a bitfield makes this change take
up no additional memory.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/fast-import.c b/fast-import.c
index 1ae125a040e6dfcf83b2cdc4aeea8fa248d2ba22..490f640036671adb898ef23f30a8000bd8bbdefc 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -220,7 +220,8 @@ struct branch
        const char *name;
        struct tree_entry branch_tree;
        uintmax_t last_commit;
        const char *name;
        struct tree_entry branch_tree;
        uintmax_t last_commit;

-       unsigned int pack_id;
+       unsigned active : 1;
+       unsigned pack_id : PACK_ID_BITS;

        unsigned char sha1[20];
 };
 
        unsigned char sha1[20];
 };
 


@@ -528,6 +529,7 @@ static struct branch *new_branch(const char *name)
        b->table_next_branch = branch_table[hc];
        b->branch_tree.versions[0].mode = S_IFDIR;
        b->branch_tree.versions[1].mode = S_IFDIR;
        b->table_next_branch = branch_table[hc];
        b->branch_tree.versions[0].mode = S_IFDIR;
        b->branch_tree.versions[1].mode = S_IFDIR;

+       b->active = 0;

        b->pack_id = MAX_PACK_ID;
        branch_table[hc] = b;
        branch_count++;
        b->pack_id = MAX_PACK_ID;
        branch_table[hc] = b;
        branch_count++;


@@ -1547,6 +1549,7 @@ static void unload_one_branch(void)
                        e = active_branches;
                        active_branches = e->active_next_branch;
                }
                        e = active_branches;
                        active_branches = e->active_next_branch;
                }

+               e->active = 0;

                e->active_next_branch = NULL;
                if (e->branch_tree.tree) {
                        release_tree_content_recursive(e->branch_tree.tree);
                e->active_next_branch = NULL;
                if (e->branch_tree.tree) {
                        release_tree_content_recursive(e->branch_tree.tree);


@@ -1559,10 +1562,13 @@ static void unload_one_branch(void)
 static void load_branch(struct branch *b)
 {
        load_tree(&b->branch_tree);
 static void load_branch(struct branch *b)
 {
        load_tree(&b->branch_tree);

-       b->active_next_branch = active_branches;
-       active_branches = b;
-       cur_active_branches++;
-       branch_load_count++;
+       if (!b->active) {
+               b->active = 1;
+               b->active_next_branch = active_branches;
+               active_branches = b;
+               cur_active_branches++;
+               branch_load_count++;
+       }

 }
 
 static void file_change_m(struct branch *b)
 }
 
 static void file_change_m(struct branch *b)