Skip to content

Commit 2e734bf

Browse files
localai-botmudler
andauthored
fix(downloader): stall timeout, resume-safe cancel, and stale-partial reaping (#10406)
* fix(downloader): stall timeout, resume-safe cancel, and stale-partial reaping Large model installs would hang forever or never finish. Three defects in the HTTP download path, all hit by big GGUF pulls over a slow or flaky link: 1. No stall timeout. The shared download client sets no body deadline (correct for streaming) but also no read-idle timeout, and the transport's IdleConnTimeout does not cover an in-flight body read. A silently-dropped TCP connection (no FIN/RST) blocked the body Read forever, freezing an install at N bytes until an external reaper killed it. Add an idle-timeout reader that closes the body after a window of zero progress (DownloadStallTimeout, default 60s), turning an indefinite hang into a fast, retryable error. A read that returns data resets the clock, so a slow-but-steady transfer is unaffected. 2. Cancellation deleted the partial. On context.Canceled the code removed the .partial file, so any frontend restart (deploy, OOM) mid-download wiped all progress and the retry restarted from zero. At slow egress, files larger than the restart interval never completed. Keep the .partial on cancel so the next attempt resumes via Range. 3. Partials leaked. Cleanup only ran on the context-cancel path, never on a stall or a SIGKILL/OOM, so abandoned .partial files accumulated and could fill the models volume. Add CleanupStalePartialFiles and reap partials older than 24h on startup. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code] * fix(downloader): discard the .partial on a deliberate user cancel Review follow-up. The previous commit kept the .partial on every cancellation so restarts could resume, but that also left a dangling partial when a user *intentionally* cancelled an install — the file lingered until the 24h reaper. Distinguish the two: cancel the gallery operation's context with a cause (downloader.ErrUserCancelled) so the download layer can tell a deliberate abort (discard the partial) from an incidental one such as a shutdown/restart (keep it for resume). Detect cancellation via the context rather than the returned error, because an HTTP request cancelled with a cause surfaces the cause error, not context.Canceled. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code] * fix(downloader): resolve gosec G122 in CleanupStalePartialFiles CI's code-scanning (gosec) flagged G122 (symlink TOCTOU) for the os.Remove call inside the filepath.WalkDir callback. Collect the stale paths during the walk and delete them afterwards instead of mutating the tree from inside the callback. Behavior is unchanged; the existing specs still pass. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code] --------- Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
1 parent 72d46c1 commit 2e734bf

8 files changed

Lines changed: 547 additions & 14 deletions

File tree

core/application/startup.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/mudler/LocalAI/core/services/storage"
2626
coreStartup "github.com/mudler/LocalAI/core/startup"
2727
"github.com/mudler/LocalAI/internal"
28+
"github.com/mudler/LocalAI/pkg/downloader"
2829
"github.com/mudler/LocalAI/pkg/signals"
2930
"github.com/mudler/LocalAI/pkg/vram"
3031

@@ -71,6 +72,16 @@ func New(opts ...config.AppOption) (*Application, error) {
7172
if err != nil {
7273
return nil, fmt.Errorf("unable to create ModelPath: %q", err)
7374
}
75+
76+
// Reap *.partial downloads abandoned by a previous run (killed mid-transfer
77+
// by an OOM/restart, or stalled before cleanup could run). The 24h window
78+
// is well beyond any legitimate in-flight download, so this never trims an
79+
// active transfer; it just stops dead partials accumulating on the volume.
80+
if removed, cErr := downloader.CleanupStalePartialFiles(options.SystemState.Model.ModelsPath, 24*time.Hour); cErr != nil {
81+
xlog.Warn("Failed to reap stale partial downloads", "error", cErr)
82+
} else if removed > 0 {
83+
xlog.Info("Reaped stale partial downloads", "count", removed)
84+
}
7485
if options.GeneratedContentDir != "" {
7586
err := os.MkdirAll(options.GeneratedContentDir, 0o750)
7687
if err != nil {

core/services/galleryop/service.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/mudler/LocalAI/core/gallery"
1212
"github.com/mudler/LocalAI/core/services/distributed"
1313
"github.com/mudler/LocalAI/core/services/messaging"
14+
"github.com/mudler/LocalAI/pkg/downloader"
1415
"github.com/mudler/LocalAI/pkg/model"
1516
"github.com/mudler/LocalAI/pkg/system"
1617
"github.com/mudler/xlog"
@@ -402,6 +403,16 @@ func (g *GalleryService) applyCancel(id string) {
402403
}
403404
}
404405

406+
// newUserCancellableContext returns a child context whose CancelFunc cancels
407+
// with the downloader.ErrUserCancelled cause. This lets the download layer
408+
// distinguish a deliberate user cancel (discard the half-downloaded .partial)
409+
// from an incidental cancellation such as process shutdown (keep the .partial
410+
// so the next run resumes via Range instead of restarting from zero).
411+
func newUserCancellableContext(parent context.Context) (context.Context, context.CancelFunc) {
412+
ctx, cancelCause := context.WithCancelCause(parent)
413+
return ctx, func() { cancelCause(downloader.ErrUserCancelled) }
414+
}
415+
405416
// storeCancellation stores a cancellation function for an operation
406417
func (g *GalleryService) storeCancellation(id string, cancelFunc context.CancelFunc) {
407418
g.Lock()
@@ -444,7 +455,7 @@ func (g *GalleryService) Start(c context.Context, cl *config.ModelConfigLoader,
444455
case op := <-g.BackendGalleryChannel:
445456
// Create context if not provided
446457
if op.Context == nil {
447-
op.Context, op.CancelFunc = context.WithCancel(c)
458+
op.Context, op.CancelFunc = newUserCancellableContext(c)
448459
g.storeCancellation(op.ID, op.CancelFunc)
449460
} else if op.CancelFunc != nil {
450461
g.storeCancellation(op.ID, op.CancelFunc)
@@ -472,7 +483,7 @@ func (g *GalleryService) Start(c context.Context, cl *config.ModelConfigLoader,
472483
case op := <-g.ModelGalleryChannel:
473484
// Create context if not provided
474485
if op.Context == nil {
475-
op.Context, op.CancelFunc = context.WithCancel(c)
486+
op.Context, op.CancelFunc = newUserCancellableContext(c)
476487
g.storeCancellation(op.ID, op.CancelFunc)
477488
} else if op.CancelFunc != nil {
478489
g.storeCancellation(op.ID, op.CancelFunc)

pkg/downloader/cancel_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package downloader_test
2+
3+
import (
4+
"context"
5+
"crypto/rand"
6+
"crypto/sha256"
7+
"errors"
8+
"fmt"
9+
"net/http"
10+
"net/http/httptest"
11+
"os"
12+
"strconv"
13+
"strings"
14+
"time"
15+
16+
. "github.com/mudler/LocalAI/pkg/downloader"
17+
. "github.com/onsi/ginkgo/v2"
18+
. "github.com/onsi/gomega"
19+
)
20+
21+
var _ = Describe("Download cancellation", func() {
22+
var filePath string
23+
24+
// streamingRangeServer serves data one small chunk at a time with a short
25+
// pause between chunks, so a context cancellation can land mid-transfer.
26+
// It honors a `bytes=N-` Range request so a second attempt can resume.
27+
streamingRangeServer := func(data []byte) *httptest.Server {
28+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
29+
if r.Method == "HEAD" {
30+
w.Header().Set("Accept-Ranges", "bytes")
31+
w.WriteHeader(http.StatusOK)
32+
return
33+
}
34+
start := 0
35+
if rh := r.Header.Get("Range"); rh != "" {
36+
_, _ = fmt.Sscanf(strings.TrimPrefix(rh, "bytes="), "%d-", &start)
37+
}
38+
w.Header().Set("Content-Length", strconv.Itoa(len(data)-start))
39+
if start > 0 {
40+
w.WriteHeader(http.StatusPartialContent)
41+
} else {
42+
w.WriteHeader(http.StatusOK)
43+
}
44+
f, _ := w.(http.Flusher)
45+
for i := start; i < len(data); i += 256 {
46+
end := i + 256
47+
if end > len(data) {
48+
end = len(data)
49+
}
50+
if _, err := w.Write(data[i:end]); err != nil {
51+
return
52+
}
53+
if f != nil {
54+
f.Flush()
55+
}
56+
time.Sleep(20 * time.Millisecond)
57+
}
58+
}))
59+
}
60+
61+
BeforeEach(func() {
62+
dir, err := os.Getwd()
63+
Expect(err).ToNot(HaveOccurred())
64+
filePath = dir + "/cancel_model"
65+
})
66+
67+
AfterEach(func() {
68+
_ = os.Remove(filePath)
69+
_ = os.Remove(filePath + ".partial")
70+
})
71+
72+
It("keeps the .partial file when the context is cancelled so the download can resume", func() {
73+
data := make([]byte, 8192)
74+
_, err := rand.Read(data)
75+
Expect(err).ToNot(HaveOccurred())
76+
server := streamingRangeServer(data)
77+
defer server.Close()
78+
79+
ctx, cancel := context.WithCancel(context.Background())
80+
go func() {
81+
time.Sleep(150 * time.Millisecond)
82+
cancel()
83+
}()
84+
85+
err = URI(server.URL).DownloadFileWithContext(ctx, filePath, "", 1, 1, func(s1, s2, s3 string, f float64) {})
86+
Expect(err).To(HaveOccurred())
87+
Expect(errors.Is(err, context.Canceled)).To(BeTrue())
88+
89+
info, statErr := os.Stat(filePath + ".partial")
90+
Expect(statErr).ToNot(HaveOccurred(),
91+
"a cancelled download must leave its .partial behind so the retry resumes instead of restarting from zero")
92+
Expect(info.Size()).To(BeNumerically(">", 0))
93+
Expect(info.Size()).To(BeNumerically("<", int64(len(data))))
94+
})
95+
96+
It("discards the .partial when the cancellation cause is ErrUserCancelled", func() {
97+
data := make([]byte, 8192)
98+
_, err := rand.Read(data)
99+
Expect(err).ToNot(HaveOccurred())
100+
server := streamingRangeServer(data)
101+
defer server.Close()
102+
103+
// A deliberate user abort: cancel WITH the ErrUserCancelled cause. The
104+
// half-finished download should not linger on disk.
105+
ctx, cancel := context.WithCancelCause(context.Background())
106+
go func() {
107+
time.Sleep(150 * time.Millisecond)
108+
cancel(ErrUserCancelled)
109+
}()
110+
111+
err = URI(server.URL).DownloadFileWithContext(ctx, filePath, "", 1, 1, func(s1, s2, s3 string, f float64) {})
112+
Expect(err).To(HaveOccurred())
113+
Expect(errors.Is(err, context.Canceled)).To(BeTrue())
114+
115+
Expect(filePath + ".partial").ToNot(BeAnExistingFile(),
116+
"a deliberate user cancel must not leave a dangling .partial behind")
117+
})
118+
119+
It("resumes from the preserved .partial after a cancellation and completes", func() {
120+
data := make([]byte, 8192)
121+
_, err := rand.Read(data)
122+
Expect(err).ToNot(HaveOccurred())
123+
sum := sha256.Sum256(data)
124+
sha := fmt.Sprintf("%x", sum)
125+
server := streamingRangeServer(data)
126+
defer server.Close()
127+
128+
// First attempt: cancel mid-stream.
129+
ctx, cancel := context.WithCancel(context.Background())
130+
go func() {
131+
time.Sleep(150 * time.Millisecond)
132+
cancel()
133+
}()
134+
err = URI(server.URL).DownloadFileWithContext(ctx, filePath, sha, 1, 1, func(s1, s2, s3 string, f float64) {})
135+
Expect(err).To(HaveOccurred())
136+
partialInfo, statErr := os.Stat(filePath + ".partial")
137+
Expect(statErr).ToNot(HaveOccurred())
138+
resumedFrom := partialInfo.Size()
139+
Expect(resumedFrom).To(BeNumerically(">", 0))
140+
141+
// Second attempt: fresh context, must resume and finish with a valid SHA.
142+
err = URI(server.URL).DownloadFileWithContext(context.Background(), filePath, sha, 1, 1, func(s1, s2, s3 string, f float64) {})
143+
Expect(err).ToNot(HaveOccurred())
144+
final, rerr := os.ReadFile(filePath)
145+
Expect(rerr).ToNot(HaveOccurred())
146+
Expect(final).To(Equal(data))
147+
})
148+
})

pkg/downloader/partial.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package downloader
2+
3+
import (
4+
"io/fs"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
"time"
9+
10+
"github.com/mudler/xlog"
11+
)
12+
13+
// PartialFileSuffix marks an in-progress download. The success path renames the
14+
// partial to its final name, so any leftover with this suffix is an unfinished
15+
// transfer.
16+
const PartialFileSuffix = ".partial"
17+
18+
// CleanupStalePartialFiles removes *.partial files under root whose last
19+
// modification is older than olderThan, returning the number removed. These are
20+
// abandoned downloads left by a process killed mid-transfer (OOM, restart) or
21+
// by a stall whose cleanup never ran; without reaping they accumulate and can
22+
// fill the models volume. A still-in-progress download touches its .partial on
23+
// every write, so a generous olderThan never trims an active transfer.
24+
//
25+
// A missing root is not an error (nothing to clean). Unreadable entries are
26+
// skipped so one bad file does not abort the whole sweep.
27+
func CleanupStalePartialFiles(root string, olderThan time.Duration) (int, error) {
28+
if _, err := os.Stat(root); err != nil {
29+
if os.IsNotExist(err) {
30+
return 0, nil
31+
}
32+
return 0, err
33+
}
34+
35+
cutoff := time.Now().Add(-olderThan)
36+
37+
// Collect candidates during the walk and delete them afterwards rather than
38+
// mutating the tree from inside the WalkDir callback (avoids the symlink
39+
// TOCTOU class flagged by gosec G122, and never removes an entry mid-walk).
40+
var stale []string
41+
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error {
42+
if walkErr != nil {
43+
return nil // skip unreadable subtree, keep going
44+
}
45+
if d.IsDir() || !strings.HasSuffix(d.Name(), PartialFileSuffix) {
46+
return nil
47+
}
48+
info, err := d.Info()
49+
if err != nil || info.ModTime().After(cutoff) {
50+
return nil
51+
}
52+
stale = append(stale, path)
53+
return nil
54+
})
55+
if err != nil {
56+
return 0, err
57+
}
58+
59+
removed := 0
60+
for _, path := range stale {
61+
if err := os.Remove(path); err != nil {
62+
xlog.Warn("failed to remove stale partial download", "file", path, "error", err)
63+
continue
64+
}
65+
removed++
66+
xlog.Info("removed stale partial download", "file", path)
67+
}
68+
return removed, nil
69+
}

pkg/downloader/partial_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package downloader_test
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"time"
7+
8+
. "github.com/mudler/LocalAI/pkg/downloader"
9+
. "github.com/onsi/ginkgo/v2"
10+
. "github.com/onsi/gomega"
11+
)
12+
13+
var _ = Describe("CleanupStalePartialFiles", func() {
14+
var root string
15+
16+
BeforeEach(func() {
17+
var err error
18+
root, err = os.MkdirTemp("", "partials")
19+
Expect(err).ToNot(HaveOccurred())
20+
})
21+
22+
AfterEach(func() {
23+
_ = os.RemoveAll(root)
24+
})
25+
26+
It("removes stale .partial files (recursively) while keeping fresh ones and completed files", func() {
27+
nested := filepath.Join(root, "llama-cpp", "models", "foo")
28+
Expect(os.MkdirAll(nested, 0755)).To(Succeed())
29+
30+
stale := filepath.Join(nested, "model.gguf.partial")
31+
fresh := filepath.Join(root, "fresh.gguf.partial")
32+
completed := filepath.Join(root, "done.gguf")
33+
for _, f := range []string{stale, fresh, completed} {
34+
Expect(os.WriteFile(f, []byte("data"), 0644)).To(Succeed())
35+
}
36+
old := time.Now().Add(-2 * time.Hour)
37+
Expect(os.Chtimes(stale, old, old)).To(Succeed())
38+
39+
removed, err := CleanupStalePartialFiles(root, time.Hour)
40+
Expect(err).ToNot(HaveOccurred())
41+
Expect(removed).To(Equal(1))
42+
43+
Expect(stale).ToNot(BeAnExistingFile())
44+
Expect(fresh).To(BeAnExistingFile())
45+
Expect(completed).To(BeAnExistingFile())
46+
})
47+
48+
It("returns no error when the root directory does not exist", func() {
49+
removed, err := CleanupStalePartialFiles(filepath.Join(root, "does-not-exist"), time.Hour)
50+
Expect(err).ToNot(HaveOccurred())
51+
Expect(removed).To(Equal(0))
52+
})
53+
})

0 commit comments

Comments
 (0)