Skip to content

Commit 8263cbc

Browse files
ZviBaratzclaude
andauthored
fix(ego-lint): narrow R-LIFE-25 auto-cleanup to D-Bus-specific disconnectSignal (#65)
## Summary - R-LIFE-25 produced 0 lint hits because the auto-cleanup suppression matched GObject signal mechanisms (`connectObject`, `disconnectObject`, `SignalTracker`, `connectSmart`) which are irrelevant for D-Bus proxy signals - Every real-world extension using `connectObject` for widget/settings signals had all `connectSignal` leak warnings suppressed - Fix narrows auto-cleanup detection to only recognize `disconnectSignal()` — the only valid D-Bus signal cleanup mechanism - Adds `dbus-signal-mixed-cleanup@test` fixture testing the exact false-negative scenario - Updates `dbus-signal-auto-cleanup@test` fixture to use `disconnectSignal` instead of `connectObject` Closes #63 ## Test plan - [x] `dbus-signal-leak@test` → FAIL (bare connectSignal, no cleanup) - [x] `dbus-signal-auto-cleanup@test` → PASS (has `disconnectSignal`) - [x] `dbus-signal-mixed-cleanup@test` → FAIL (GObject cleanup doesn't count for D-Bus) - [x] Full test suite: 601 passed, 0 failed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 99c89c0 commit 8263cbc

File tree

7 files changed

+51
-16
lines changed

7 files changed

+51
-16
lines changed

skills/ego-lint/references/rules-reference.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,12 +2070,12 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
20702070
### R-LIFE-25: D-Bus proxy signal leak (connectSignal without disconnectSignal)
20712071
- **Severity**: blocking
20722072
- **Checked by**: check-lifecycle.py
2073-
- **Rule**: `proxy.connectSignal('...')` calls where the return value is not stored (no `=` or `.push()` before `.connectSignal` on the line) and no auto-cleanup mechanism (`disconnectObject`, `connectObject`, `SignalTracker`, `connectSmart`) is present in the extension.
2073+
- **Rule**: `proxy.connectSignal('...')` calls where the return value is not stored (no `=` or `.push()` before `.connectSignal` on the line) and no `disconnectSignal()` call is present in the extension. GObject cleanup mechanisms (`connectObject`, `disconnectObject`, `SignalTracker`, `connectSmart`) do NOT clean up D-Bus proxy signals and are not considered valid cleanup.
20742074
- **Rationale**: Without a stored signal ID, there is no way to call `proxy.disconnectSignal(id)` later. These D-Bus signal handlers accumulate across enable/disable cycles, causing duplicate callbacks and memory leaks. This is distinct from GObject `.connect()` — `connectSignal()` is specific to `Gio.DBusProxy` for subscribing to D-Bus signals (e.g., `PropertiesChanged`, `NameOwnerChanged`).
2075-
- **Fix**: Store the return value and call `proxy.disconnectSignal(id)` in `disable()`, or use `connectObject()`/`disconnectObject()` for auto-cleanup.
2075+
- **Fix**: Store the return value and call `proxy.disconnectSignal(id)` in `disable()`.
20762076
- **Scope exclusions**: prefs.js (manages own lifecycle), `service/` directory (daemon lifecycle).
20772077
- **Known limitations**: Multi-line `.connectSignal(\n'...'` calls are not detected (per-line scanning).
2078-
- **Tested by**: `tests/fixtures/dbus-signal-leak@test/`, `tests/fixtures/dbus-signal-auto-cleanup@test/`, `tests/fixtures/dbus-signal-array-storage@test/`
2078+
- **Tested by**: `tests/fixtures/dbus-signal-leak@test/`, `tests/fixtures/dbus-signal-auto-cleanup@test/`, `tests/fixtures/dbus-signal-array-storage@test/`, `tests/fixtures/dbus-signal-mixed-cleanup@test/`
20792079
20802080
### R-LIFE-26: Module-scope mutable state not cleared in disable()
20812081
- **Severity**: advisory

skills/ego-lint/scripts/check-lifecycle.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,17 +1286,16 @@ def check_dbus_signal_leak(ext_dir):
12861286
return
12871287

12881288
bare_connects = []
1289-
has_auto_cleanup = False
1289+
has_dbus_disconnect = False
12901290

12911291
for filepath in js_files:
12921292
content = strip_comments(read_file(filepath))
12931293
rel = os.path.relpath(filepath, ext_dir)
12941294

1295-
# Extension-wide auto-cleanup detection
1296-
if (re.search(r'\.disconnectObject\s*\(', content) or
1297-
re.search(r'\.connectObject\s*\(', content) or
1298-
re.search(r'\b(SignalTracker|SignalManager|connectSmart|disconnectSmart)\b', content)):
1299-
has_auto_cleanup = True
1295+
# D-Bus-specific cleanup detection: only disconnectSignal counts
1296+
# (connectObject/disconnectObject manage GObject signals, not D-Bus signals)
1297+
if re.search(r'\.disconnectSignal\s*\(', content):
1298+
has_dbus_disconnect = True
13001299

13011300
prev_stripped = ''
13021301
for lineno, line in enumerate(content.splitlines(), 1):
@@ -1325,16 +1324,16 @@ def check_dbus_signal_leak(ext_dir):
13251324
bare_connects.append(f"{rel}:{lineno}")
13261325
prev_stripped = stripped
13271326

1328-
if bare_connects and not has_auto_cleanup:
1327+
if bare_connects and not has_dbus_disconnect:
13291328
count = len(bare_connects)
13301329
locs = ', '.join(bare_connects[:5])
13311330
suffix = f' (and {count - 5} more)' if count > 5 else ''
13321331
result("FAIL", "lifecycle/dbus-signal-leak",
13331332
f"{count} bare proxy.connectSignal() without stored ID "
1334-
f"and no disconnectObject/connectObject cleanup: {locs}{suffix}")
1333+
f"and no disconnectSignal() cleanup: {locs}{suffix}")
13351334
elif bare_connects:
13361335
result("PASS", "lifecycle/dbus-signal-leak",
1337-
"Bare connectSignal() found but auto-cleanup mechanism present")
1336+
"Bare connectSignal() found but disconnectSignal() cleanup present")
13381337
# If no bare connectSignal calls, skip silently
13391338

13401339

tests/assertions/dbus-signal-leak.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,10 @@ run_lint "dbus-signal-array-storage@test"
2020
assert_exit_code "exits with 0 (array storage is not bare connect)" 0
2121
assert_output_not_contains "no dbus-signal-leak FP on array storage" "\[FAIL\].*lifecycle/dbus-signal-leak"
2222
echo ""
23+
24+
echo "=== dbus-signal-mixed-cleanup ==="
25+
run_lint "dbus-signal-mixed-cleanup@test"
26+
assert_exit_code "exits with 1 (has failures)" 1
27+
assert_output_contains "fails despite GObject cleanup" "\[FAIL\].*lifecycle/dbus-signal-leak"
28+
assert_output_not_contains "no false auto-cleanup pass" "\[PASS\].*lifecycle/dbus-signal-leak"
29+
echo ""

tests/fixtures/dbus-signal-auto-cleanup@test/extension.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,13 @@ export default class DbusAutoCleanupExtension extends Extension {
99
'org.mpris.MediaPlayer2.Player', null);
1010
this._proxy.connectSignal('PropertiesChanged', () => this._onProps());
1111

12-
this._settings = this.getSettings();
13-
this._settings.connectObject('changed::mode', () => {}, this);
12+
this._seekedId = this._proxy.connectSignal('Seeked', () => {});
1413
}
1514

1615
_onProps() {}
1716

1817
disable() {
19-
this._settings.disconnectObject(this);
18+
this._proxy.disconnectSignal(this._seekedId);
2019
this._proxy = null;
21-
this._settings = null;
2220
}
2321
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
SPDX-License-Identifier: GPL-2.0-or-later
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
2+
import Gio from 'gi://Gio';
3+
4+
export default class DbusMixedCleanupExtension extends Extension {
5+
enable() {
6+
this._proxy = Gio.DBusProxy.new_for_bus_sync(
7+
Gio.BusType.SESSION, 0, null,
8+
'org.mpris.MediaPlayer2', '/org/mpris/MediaPlayer2',
9+
'org.mpris.MediaPlayer2.Player', null);
10+
this._proxy.connectSignal('PropertiesChanged', () => this._onProps());
11+
12+
this._settings = this.getSettings();
13+
this._settings.connectObject('changed::mode', () => {}, this);
14+
}
15+
16+
_onProps() {}
17+
18+
disable() {
19+
this._settings.disconnectObject(this);
20+
this._proxy = null;
21+
this._settings = null;
22+
}
23+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"uuid": "dbus-signal-mixed-cleanup@test",
3+
"name": "D-Bus Signal Mixed Cleanup Test",
4+
"shell-version": ["45"],
5+
"description": "Fixture for R-LIFE-25: bare connectSignal with GObject-only cleanup",
6+
"url": "https://example.com"
7+
}

0 commit comments

Comments
 (0)