fast-import: Avoid infinite loop after reset

PATCH In review Shawn O. Pearce 1 participant 1 message Jun 15, 2016

Share & tools

Shawn O. Pearce

author

2016-06-15 22:42 UTC #0 lore raw

PATCH 1/2 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 <[email protected]>

---

This two patch series was made on top of maint and is also pullable from my fast-import fork on repo.or.cz. Merging it with master/next may require a minor evil merge as Nico's API changes there might be conflicting.

 fast-import.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

fast-import.c +11 -5

index 1ae125a..490f640 100644
@@ -220,7 +220,8 @@ struct branch
 	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];
 };

@@ -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->active = 0;
 	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 = 0;
 		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);
-	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)

Signature · 1 line

1.5.0.3.862.g71037