Skip to content

Commit 40dc756

Browse files
committed
tasklog/percentage_task.go: add Complete() method
In commit e90d54d of PR git-lfs#2329 the "git/githistory/log" package was added, including the PercentageTask structure and associated methods, but unlike the WaitingTask which was added at the same time, no Complete() method was defined for the PercentageTask structure. (Note that the "git/githistory/log" package was later renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747.) Other task types such as the ListTask and SimpleTask structures (introduced in commit 31ffeb9 of PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756, respectively) provide a Complete() method, and the Meter task type of the "tq" package, which implemented the Task interface in commit 7c0f9e2 of PR git-lfs#2732, provides an equivalent Finish() method. These methods allow the caller to explicitly close the channel returned by the Updates() method, on which the anonymous goroutine started by the Logger.consume() method for the task is waiting, in a "range" loop on the channel in the Logger.logTask() method. One key use of the PercentageTask structure is in the methods of the Rewriter structure from the "git/githistory" package. It is initialized with the number of commits to be rewritten during a "git lfs migrate" command's traversal of a Git repository's history. As each commit is rewritten, the Count() method is called to increment the percentage completed value. When every commit has been rewritten, the count reaches the expected total, and the Count() method closes the channel, thus allowing the task receiving updates to finish and exit its goroutine. Under exceptional circumstances, though, the Rewrite() method of the Rewriter structure may never finish iterating through all the expected commits, and return in error or via a panic call. When this happens, the goroutine waiting on the PercentageTask's updates channel can never exit, leading to a hung process which never fully exits. In order to allow the Rewriter's Rewrite() method to define a deferred function which will always be called when it returns, under any circumstances, we add a Complete() method for the PercentageTask structure which sets the number of completed elements to the expected total in an atomic swap, and then closes the updates channel if the number of completed elements was previously less than the expected total. We also add a test to validate this new method's behaviour, and update the existing TestPercentageTaskCallsDoneWhenComplete() function to also confirm that calling the new Complete() method after the number of completed elements has reached the expected total does not cause a second attempt to close the updates channel.
1 parent 3d872ab commit 40dc756

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

tasklog/percentage_task.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ func (c *PercentageTask) Entry(update string) {
7878
}
7979
}
8080

81+
// Complete notes that the task is completed by setting the number of
82+
// completed elements to the total number of elements, and if necessary
83+
// closing the Updates channel, which yields the logger to the next Task.
84+
func (c *PercentageTask) Complete() {
85+
if count := atomic.SwapUint64(&c.n, c.total); count < c.total {
86+
close(c.ch)
87+
}
88+
}
89+
8190
// Updates implements Task.Updates and returns a channel which is written to
8291
// when the state of this task changes, and closed when the task is completed.
8392
func (c *PercentageTask) Updates() <-chan *Update {

tasklog/percentage_task_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,37 @@ func TestPercentageTaskCallsDoneWhenComplete(t *testing.T) {
5050
if _, ok := <-task.Updates(); ok {
5151
t.Fatalf("expected channel to be closed")
5252
}
53+
54+
defer func() {
55+
if err := recover(); err != nil {
56+
t.Fatal("tasklog: expected *PercentageTask.Complete() to not panic")
57+
}
58+
}()
59+
60+
task.Complete()
61+
}
62+
63+
func TestPercentageTaskCompleteClosesUpdates(t *testing.T) {
64+
task := NewPercentageTask("example", 10)
65+
66+
select {
67+
case v, ok := <-task.Updates():
68+
if ok {
69+
assert.Equal(t, "example: 0% (0/10)", v.S)
70+
} else {
71+
t.Fatal("expected channel to be open")
72+
}
73+
default:
74+
}
75+
76+
assert.EqualValues(t, 7, task.Count(7))
77+
assert.Equal(t, "example: 70% (7/10)", (<-task.Updates()).S)
78+
79+
task.Complete()
80+
81+
if _, ok := <-task.Updates(); ok {
82+
t.Fatalf("expected channel to be closed")
83+
}
5384
}
5485

5586
func TestPercentageTaskIsThrottled(t *testing.T) {

0 commit comments

Comments
 (0)