Skip to content

Commit ee53818

Browse files
MaxGhenisclaude
andauthored
Fix libsignal temp dir leak from signal-cli subprocess churn (#28)
Every signal-cli invocation extracts ~21MB of native libraries into a fresh libsignal* directory under java.io.tmpdir. Receive polling spawns one JVM every few seconds, and runs that overran their context deadline were SIGKILLed, stranding the extraction dir. Leaked dirs accumulated until the disk filled (87GB / 4,455 dirs observed in #27). Three defenses, verified against signal-cli 0.14.1: - Confine: each invocation now runs with TMPDIR and java.io.tmpdir (appended last to SIGNAL_CLI_OPTS, so it wins) pointed at a private per-run dir under <configDir>/tmp, removed as soon as the process exits — leaks are structurally impossible even on SIGKILL. - Graceful cancel: context cancellation now sends SIGTERM first with a 3s WaitDelay before the hard kill, so the JVM's own shutdown hooks (which include libsignal temp cleanup) get a chance to run. - Sweep: the app-owned tmp root is swept at startup and every 10 minutes for entries no live run can still own (crash backstop), and libsignal* dirs older than 24h that earlier builds leaked into the system temp dir are reaped on the same cadence so existing installs recover disk space without manual cleanup. Opt out with OPENMESSAGES_SIGNAL_TMP_SWEEP=0. Fixes #27 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 7964aa7 commit ee53818

3 files changed

Lines changed: 459 additions & 1 deletion

File tree

internal/signallive/client.go

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"strconv"
2020
"strings"
2121
"sync"
22+
"syscall"
2223
"time"
2324

2425
"github.com/rs/zerolog"
@@ -54,22 +55,59 @@ var (
5455
runSignalCLI = func(ctx context.Context, configDir string, args ...string) ([]byte, error) {
5556
commandArgs := append([]string{"--config", configDir}, args...)
5657
cmd := exec.CommandContext(ctx, signalCLIExecutable(), commandArgs...)
58+
tmpDir, cleanupTmp, tmpErr := newSignalRunTmpDir(configDir)
59+
if tmpErr == nil {
60+
cmd.Env = signalCLIEnv(os.Environ(), tmpDir)
61+
defer cleanupTmp()
62+
}
63+
configureSignalCancel(cmd)
5764
return cmd.CombinedOutput()
5865
}
5966

6067
startSignalLink = func(ctx context.Context, configDir string) (io.ReadCloser, func() error, error) {
6168
cmd := exec.CommandContext(ctx, "script", "-q", "/dev/null", signalCLIExecutable(), "--config", configDir, "link", "-n", "OpenMessage")
69+
tmpDir, cleanupTmp, tmpErr := newSignalRunTmpDir(configDir)
70+
if tmpErr == nil {
71+
cmd.Env = signalCLIEnv(os.Environ(), tmpDir)
72+
}
73+
configureSignalCancel(cmd)
6274
stdout, err := cmd.StdoutPipe()
6375
if err != nil {
76+
if tmpErr == nil {
77+
cleanupTmp()
78+
}
6479
return nil, nil, err
6580
}
6681
if err := cmd.Start(); err != nil {
82+
if tmpErr == nil {
83+
cleanupTmp()
84+
}
6785
return nil, nil, err
6886
}
69-
return stdout, cmd.Wait, nil
87+
wait := func() error {
88+
err := cmd.Wait()
89+
if tmpErr == nil {
90+
cleanupTmp()
91+
}
92+
return err
93+
}
94+
return stdout, wait, nil
7095
}
7196
)
7297

98+
// configureSignalCancel asks for a graceful stop before the hard kill so
99+
// the JVM gets a chance to run its shutdown hooks (which include libsignal
100+
// temp cleanup) and flush output when a context deadline fires.
101+
func configureSignalCancel(cmd *exec.Cmd) {
102+
cmd.Cancel = func() error {
103+
if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
104+
return cmd.Process.Kill()
105+
}
106+
return nil
107+
}
108+
cmd.WaitDelay = 3 * time.Second
109+
}
110+
73111
type Callbacks struct {
74112
OnConversationsChange func()
75113
OnIncomingMessage func(*db.Message)
@@ -165,6 +203,7 @@ type Bridge struct {
165203
importedMessages int
166204
}
167205
lastReceiveRecoveryAt int64
206+
lastTmpSweep time.Time
168207
}
169208

170209
type signalReceiveRecoveryRecord struct {
@@ -326,6 +365,14 @@ func New(configDir string, store *db.Store, logger zerolog.Logger, callbacks Cal
326365
contactByACI: map[string]string{},
327366
}
328367
bridge.account = bridge.firstStoredAccount()
368+
bridge.lastTmpSweep = now()
369+
go sweepSignalTmpRoot(logger, configDir, signalRunTmpMaxAge)
370+
if bridge.account != "" {
371+
// Only a paired install can have produced libsignal litter, so the
372+
// legacy system-temp sweep stays scoped to installs that ran the
373+
// bridge before per-run temp dirs existed.
374+
go sweepLegacyLibsignalTemp(logger)
375+
}
329376
return bridge, nil
330377
}
331378

@@ -839,6 +886,8 @@ func (b *Bridge) startReceiveLoop(account string, requestSync bool) {
839886
default:
840887
}
841888

889+
b.maybeSweepTmp()
890+
842891
callCtx, callCancel := context.WithTimeout(ctx, time.Duration(receiveTimeoutSeconds+3)*time.Second)
843892
b.commandMu.Lock()
844893
output, err := runSignalCLI(callCtx, b.configDir, "-a", probedAccount, "--output", "json", "receive", "--timeout", strconv.Itoa(receiveTimeoutSeconds), "--max-messages", strconv.Itoa(receiveMaxMessages))
@@ -901,6 +950,25 @@ func (b *Bridge) startReceiveLoop(account string, requestSync bool) {
901950
}
902951
}
903952

953+
// maybeSweepTmp runs the crash-backstop sweeps at most once per
954+
// signalTmpSweepInterval. Normal runs clean up after themselves, so the
955+
// app-tmp sweep usually finds nothing; the legacy sweep keeps reaping
956+
// system-temp dirs leaked by older builds as they age past the gate.
957+
func (b *Bridge) maybeSweepTmp() {
958+
b.mu.Lock()
959+
due := now().Sub(b.lastTmpSweep) >= signalTmpSweepInterval
960+
if due {
961+
b.lastTmpSweep = now()
962+
}
963+
b.mu.Unlock()
964+
if due {
965+
go func() {
966+
sweepSignalTmpRoot(b.logger, b.configDir, signalRunTmpMaxAge)
967+
sweepLegacyLibsignalTemp(b.logger)
968+
}()
969+
}
970+
}
971+
904972
func (b *Bridge) refreshMetadataAndReplay(account string) {
905973
b.refreshContacts()
906974
b.refreshGroupNames()

internal/signallive/tmpdir.go

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
package signallive
2+
3+
import (
4+
"io/fs"
5+
"os"
6+
"path/filepath"
7+
"strconv"
8+
"strings"
9+
"time"
10+
11+
"github.com/rs/zerolog"
12+
)
13+
14+
// signal-cli runs on the JVM; libsignal-client extracts ~21MB of native
15+
// libraries into a fresh java.io.tmpdir subdirectory (libsignal*) on every
16+
// invocation and does not reliably remove it on exit. Because the bridge
17+
// invokes signal-cli repeatedly (receive polling, sends, metadata refresh),
18+
// leaked directories can fill the disk (issue #27). Three defenses:
19+
//
20+
// 1. Every invocation runs with TMPDIR/java.io.tmpdir pointed at a private
21+
// per-run directory under <configDir>/tmp, removed as soon as the
22+
// process exits — no accumulation by construction.
23+
// 2. <configDir>/tmp is swept at startup and periodically for entries old
24+
// enough that no live invocation can still own them (crash backstop).
25+
// 3. A one-time sweep of the system temp dir removes libsignal* dirs
26+
// abandoned by earlier versions, so existing installs recover their
27+
// disk space without manual cleanup.
28+
29+
const (
30+
// signalRunTmpMaxAge must exceed the longest legitimate signal-cli run.
31+
// Receive polls finish in seconds and sends within sendTimeout, but a
32+
// `link` session can stay open while the user fetches their phone, so
33+
// keep a generous margin.
34+
signalRunTmpMaxAge = 30 * time.Minute
35+
36+
signalTmpSweepInterval = 10 * time.Minute
37+
38+
// legacyLibsignalMaxAge gates the one-time system temp dir sweep. A day
39+
// is far older than any live signal-cli process while young enough to
40+
// recover a runaway leak quickly.
41+
legacyLibsignalMaxAge = 24 * time.Hour
42+
43+
signalTmpSweepEnvVar = "OPENMESSAGES_SIGNAL_TMP_SWEEP"
44+
)
45+
46+
func signalTmpRoot(configDir string) string {
47+
return filepath.Join(configDir, "tmp")
48+
}
49+
50+
// newSignalRunTmpDir creates a private temp dir for one signal-cli
51+
// invocation. The returned cleanup removes it; callers must invoke cleanup
52+
// only after the subprocess has exited.
53+
func newSignalRunTmpDir(configDir string) (string, func(), error) {
54+
root := signalTmpRoot(configDir)
55+
if err := os.MkdirAll(root, 0o700); err != nil {
56+
return "", nil, err
57+
}
58+
dir, err := os.MkdirTemp(root, "run-")
59+
if err != nil {
60+
return "", nil, err
61+
}
62+
return dir, func() { _ = os.RemoveAll(dir) }, nil
63+
}
64+
65+
// signalCLIEnv rebases the subprocess's temp space onto dir. TMPDIR covers
66+
// signal-cli itself plus anything it spawns; java.io.tmpdir (appended last
67+
// to SIGNAL_CLI_OPTS so it wins) covers JVMs that derive their temp dir from
68+
// the platform default instead of TMPDIR.
69+
func signalCLIEnv(base []string, dir string) []string {
70+
javaOpt := "-Djava.io.tmpdir=" + dir
71+
opts := javaOpt
72+
env := make([]string, 0, len(base)+2)
73+
for _, kv := range base {
74+
switch {
75+
case strings.HasPrefix(kv, "TMPDIR="):
76+
continue
77+
case strings.HasPrefix(kv, "SIGNAL_CLI_OPTS="):
78+
if existing := strings.TrimSpace(strings.TrimPrefix(kv, "SIGNAL_CLI_OPTS=")); existing != "" {
79+
opts = existing + " " + javaOpt
80+
}
81+
continue
82+
}
83+
env = append(env, kv)
84+
}
85+
return append(env, "TMPDIR="+dir, "SIGNAL_CLI_OPTS="+opts)
86+
}
87+
88+
func signalTmpSweepDisabled() bool {
89+
return strings.TrimSpace(os.Getenv(signalTmpSweepEnvVar)) == "0"
90+
}
91+
92+
// sweepSignalTmpRoot removes entries under <configDir>/tmp older than
93+
// maxAge. The bridge owns this directory outright, so every stale entry is
94+
// a leftover from a crashed run regardless of its name.
95+
func sweepSignalTmpRoot(logger zerolog.Logger, configDir string, maxAge time.Duration) {
96+
if signalTmpSweepDisabled() {
97+
return
98+
}
99+
removed, bytes := sweepDirEntries(signalTmpRoot(configDir), maxAge, func(string) bool { return true })
100+
if removed > 0 {
101+
logger.Info().
102+
Int("dirs", removed).
103+
Str("reclaimed", humanBytes(bytes)).
104+
Msg("Swept stale signal-cli temp dirs left by interrupted runs")
105+
}
106+
}
107+
108+
// sweepLegacyLibsignalTemp clears libsignal* dirs that earlier OpenMessage
109+
// versions leaked into the system temp dir (issue #27). Only dirs beyond
110+
// legacyLibsignalMaxAge are touched: anything that old cannot belong to a
111+
// live signal-cli run, and libsignal re-extracts on demand if another app
112+
// somehow still references one.
113+
func sweepLegacyLibsignalTemp(logger zerolog.Logger) {
114+
if signalTmpSweepDisabled() {
115+
return
116+
}
117+
removed, bytes := sweepDirEntries(os.TempDir(), legacyLibsignalMaxAge, func(name string) bool {
118+
return strings.HasPrefix(name, "libsignal")
119+
})
120+
if removed > 0 {
121+
logger.Warn().
122+
Int("dirs", removed).
123+
Str("reclaimed", humanBytes(bytes)).
124+
Msg("Removed libsignal temp dirs leaked by earlier versions (see issue #27)")
125+
}
126+
}
127+
128+
func sweepDirEntries(root string, maxAge time.Duration, match func(name string) bool) (int, int64) {
129+
entries, err := os.ReadDir(root)
130+
if err != nil {
131+
return 0, 0
132+
}
133+
cutoff := now().Add(-maxAge)
134+
removed := 0
135+
var reclaimed int64
136+
for _, entry := range entries {
137+
if !entry.IsDir() || !match(entry.Name()) {
138+
continue
139+
}
140+
info, err := entry.Info()
141+
if err != nil || info.ModTime().After(cutoff) {
142+
continue
143+
}
144+
path := filepath.Join(root, entry.Name())
145+
size := dirSize(path)
146+
if err := os.RemoveAll(path); err != nil {
147+
continue
148+
}
149+
removed++
150+
reclaimed += size
151+
}
152+
return removed, reclaimed
153+
}
154+
155+
func dirSize(root string) int64 {
156+
var total int64
157+
_ = filepath.WalkDir(root, func(_ string, d fs.DirEntry, err error) error {
158+
if err != nil || d.IsDir() {
159+
return nil
160+
}
161+
if info, err := d.Info(); err == nil {
162+
total += info.Size()
163+
}
164+
return nil
165+
})
166+
return total
167+
}
168+
169+
func humanBytes(n int64) string {
170+
const unit = 1024
171+
if n < unit {
172+
return strconv.FormatInt(n, 10) + " B"
173+
}
174+
div, exp := int64(unit), 0
175+
for m := n / unit; m >= unit; m /= unit {
176+
div *= unit
177+
exp++
178+
}
179+
return strconv.FormatFloat(float64(n)/float64(div), 'f', 1, 64) + " " + string("KMGTPE"[exp]) + "iB"
180+
}

0 commit comments

Comments
 (0)