From 69a0a231ac306740cd456eaccf4f73349ac4dcc4 Mon Sep 17 00:00:00 2001 From: Pablo Perez De Angelis Date: Thu, 29 Jan 2026 23:13:03 -0300 Subject: [PATCH 1/2] fix(net): plug memory leaks in device monitor teardown path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On repeated enable/disable cycles, gnome-shell RSS grew steadily. Three root causes were identified: 1. DeviceMonitor.deinit() never disconnected per-device state-changed signals. Those handlers held references to the monitor instance and kept scheduling GLib timeouts on a half-torn-down object, leaking both the signal connections and the timeout sources. 2. DeviceMonitor.deinit() was never called at all. DevicePresenter had no deinit() method, so the entire teardown chain stopped at AppController — the monitor stayed alive with all its NM client signals still connected. 3. The _devices map was never cleared, accumulating stale GObject references to NM.Device instances for interfaces that had been removed (e.g. Docker veth pairs being torn down). Additionally, DeviceMonitor.init() subscribed to 9 NM client signals when only 5 are needed. any-device-added/removed are supersets of device-added/removed and caused double reloads. connection-added/removed fire on profile config edits, not connectivity changes, adding noise without value. Changes: - DeviceMonitor.deinit(): disconnect per-device state-changed signals, cancel any pending debounce timer, and clear the _devices map before disconnecting NM client signals. - DeviceMonitor._loadDevices(): clear _devices map before rebuilding it so removed interfaces do not persist as stale entries. Skip interfaces not managed by NetworkManager (null from get_device_by_iface). - DeviceMonitor.init(): remove any-device-added, any-device-removed, connection-added, and connection-removed signals. Keep device-added, device-removed, active-connection-added, active-connection-removed, and notify::metered (5 total). - DeviceMonitor: add _scheduleReload() to debounce rapid NM signal bursts (e.g. Docker container start/stop) into a single 500ms deferred _loadDevices() call. - DevicePresenter: add deinit() that unsubscribes from the device-reset broadcaster and calls _deviceMonitor.deinit(). Convert resetDeviceStats to an arrow property so the same reference works for both subscribe and unsubscribe (consistent with AppController's handler pattern). - AppController.deinit(): call _devicePresenter.deinit() between saveStats() and _appSettingsModel.deinit() to wire the full teardown chain. --- lib/AppController.ts | 1 + lib/net/DeviceMonitor.ts | 51 ++++++++++++++++++++++++++------------ lib/net/DevicePresenter.ts | 15 ++++++++--- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/lib/AppController.ts b/lib/AppController.ts index 91bd354..5ee52c7 100644 --- a/lib/AppController.ts +++ b/lib/AppController.ts @@ -47,6 +47,7 @@ export class AppController { Broadcasters.titleClickedMessageBroadcaster?.unsubscribe(this.onRightClick); this._appSettingsModel.unsubscribe(this.onSettingChanged); this._devicePresenter.saveStats(); + this._devicePresenter.deinit(); this._appSettingsModel.deinit(); this.uninstallTimers(); } diff --git a/lib/net/DeviceMonitor.ts b/lib/net/DeviceMonitor.ts index c909c59..8e2fd26 100644 --- a/lib/net/DeviceMonitor.ts +++ b/lib/net/DeviceMonitor.ts @@ -34,6 +34,7 @@ export class DeviceMonitor { private _defaultGw: string = ""; private _netMgrSignals: number[] = []; private _netMgrStateChangeSignals: SignalConnection[] = []; + private _reloadTimeout: number | null = null; constructor(private _logger: Logger) { this._textDecoder = new TextDecoder(); @@ -161,24 +162,12 @@ export class DeviceMonitor { * It connects to the network manager signals to get the device changes. */ init(): void { - this._netMgrSignals.push( - this._client.connect("any-device-added", this._deviceChanged.bind(this)) - ); - this._netMgrSignals.push( - this._client.connect("any-device-removed", this._deviceChanged.bind(this)) - ); this._netMgrSignals.push( this._client.connect("device-added", this._deviceChanged.bind(this)) ); this._netMgrSignals.push( this._client.connect("device-removed", this._deviceChanged.bind(this)) ); - this._netMgrSignals.push( - this._client.connect("connection-added", this._connectionChanged.bind(this)) - ); - this._netMgrSignals.push( - this._client.connect("connection-removed", this._connectionChanged.bind(this)) - ); this._netMgrSignals.push( this._client.connect("active-connection-added", this._connectionChanged.bind(this)) ); @@ -197,10 +186,16 @@ export class DeviceMonitor { * It disconnects from the network manager signals. */ deinit(): void { + if (this._reloadTimeout) { + GLib.source_remove(this._reloadTimeout); + this._reloadTimeout = null; + } + this._disconnectDeviceStateChangeSignals(); this._netMgrSignals.forEach((sigId) => { this._client.disconnect(sigId); }); this._netMgrSignals = []; + this._devices = {}; } /** @@ -212,6 +207,7 @@ export class DeviceMonitor { private _loadDevices(): void { // disconnect "state-changed" signals of previously stored devices. this._disconnectDeviceStateChangeSignals(); + this._devices = {}; const fileContent = GLib.file_get_contents("/proc/net/dev"); const lines = this._textDecoder.decode(fileContent[1]).split("\n"); @@ -229,6 +225,10 @@ export class DeviceMonitor { } for (const name of devices) { const deviceObj = this._client.get_device_by_iface(name); + if (deviceObj == null) { + // Skip interfaces not managed by NetworkManager (e.g. Docker veth pairs) + continue; + } const addresses = this._getIPAddress(deviceObj, GLib.SYSDEF_AF_INET); const type = this.getDeviceType(deviceObj); const active = this.isActive(deviceObj); @@ -298,12 +298,27 @@ export class DeviceMonitor { this._netMgrStateChangeSignals = []; } + /** + * Schedules a debounced reload of devices. + * Multiple rapid signal callbacks are collapsed into a single reload. + */ + private _scheduleReload(): void { + if (this._reloadTimeout) { + GLib.source_remove(this._reloadTimeout); + } + this._reloadTimeout = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 500, () => { + this._reloadTimeout = null; + this._loadDevices(); + return GLib.SOURCE_REMOVE; + }); + } + /** * Handles the device state changed signal. * It reloads the devices. */ private _deviceStateChanged(): void { - this._loadDevices(); + this._scheduleReload(); } /** @@ -311,7 +326,7 @@ export class DeviceMonitor { * It reloads the devices. */ private _deviceChanged(): void { - this._loadDevices(); + this._scheduleReload(); } /** @@ -319,7 +334,7 @@ export class DeviceMonitor { * It reloads the devices. */ private _connectionChanged(): void { - this._loadDevices(); + this._scheduleReload(); } /** @@ -328,8 +343,12 @@ export class DeviceMonitor { * @param family - IP address family * @returns IP addresses of the device. */ - private _getIPAddress(device: NMDevice, family: number): string[] { + private _getIPAddress(device: NMDevice | null, family: number): string[] { const addresses: string[] = []; + if (device == null) { + addresses[0] = "-"; + return addresses; + } let ipConfig: NM.IPConfig | null = null; if (family == GLib.SYSDEF_AF_INET) ipConfig = device.get_ip4_config(); else ipConfig = device.get_ip6_config(); diff --git a/lib/net/DevicePresenter.ts b/lib/net/DevicePresenter.ts index 45a7125..60df33d 100644 --- a/lib/net/DevicePresenter.ts +++ b/lib/net/DevicePresenter.ts @@ -115,7 +115,16 @@ export class DevicePresenter { } this._stats = stats; } - Broadcasters.deviceResetMessageBroadcaster?.subscribe(this.resetDeviceStats.bind(this)); + Broadcasters.deviceResetMessageBroadcaster?.subscribe(this.resetDeviceStats); + } + + /** + * Deinitializes the device presenter. + * It unsubscribes from broadcasters and deinitializes the device monitor. + */ + deinit(): void { + Broadcasters.deviceResetMessageBroadcaster?.unsubscribe(this.resetDeviceStats); + this._deviceMonitor.deinit(); } /** @@ -438,7 +447,7 @@ export class DevicePresenter { * Reset the stats for a specific device. * It updates the stats in memory and also in the settings. */ - resetDeviceStats({ name }: { name: string }): void { + resetDeviceStats = ({ name }: { name: string }): void => { const now = new Date(); this._logger.info(`Resetting the device ${name} at ${now.toString()}`); if (this._stats[name]) { @@ -457,7 +466,7 @@ export class DevicePresenter { }; this._appSettingsModel.replaceDeviceInfo(name, deviceLogs); } - } + }; /** * Reset all devices stats. Remove all the devices which are not active. From 35b0ded1cc151660c654860e4711ad4fef53cdf4 Mon Sep 17 00:00:00 2001 From: Pablo Perez De Angelis Date: Tue, 3 Feb 2026 18:40:41 -0300 Subject: [PATCH 2/2] fix(net): guard against null devices and IP config errors in device monitor Wrap device enumeration in try/catch to handle transient NM errors from short-lived interfaces (Docker veth pairs, etc.) that can disappear between /proc/net/dev read and NM query. Also guard get_ip4_config / get_ip6_config calls which can throw on devices in transitional states. Add skipLibCheck to tsconfig.json to avoid build failures from conflicting ambient @types declarations. --- lib/net/DeviceMonitor.ts | 50 ++++++++++++++++++++++++---------------- tsconfig.json | 3 ++- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/net/DeviceMonitor.ts b/lib/net/DeviceMonitor.ts index 8e2fd26..7f617df 100644 --- a/lib/net/DeviceMonitor.ts +++ b/lib/net/DeviceMonitor.ts @@ -224,25 +224,29 @@ export class DeviceMonitor { devices.push(deviceName); } for (const name of devices) { - const deviceObj = this._client.get_device_by_iface(name); - if (deviceObj == null) { - // Skip interfaces not managed by NetworkManager (e.g. Docker veth pairs) - continue; + try { + const deviceObj = this._client.get_device_by_iface(name); + if (deviceObj == null) { + // Skip interfaces not managed by NetworkManager (e.g. Docker veth pairs) + continue; + } + const addresses = this._getIPAddress(deviceObj, GLib.SYSDEF_AF_INET); + const type = this.getDeviceType(deviceObj); + const active = this.isActive(deviceObj); + const metered = this.isMetered(deviceObj); + const dummy = this.isDummy(deviceObj); + this._devices[name] = { + name, + type, + device: deviceObj, + ip: addresses[0] || "", + active, + metered, + dummy + }; + } catch (e) { + this._logger.info(`Skipping device '${name}': ${e}`); } - const addresses = this._getIPAddress(deviceObj, GLib.SYSDEF_AF_INET); - const type = this.getDeviceType(deviceObj); - const active = this.isActive(deviceObj); - const metered = this.isMetered(deviceObj); - const dummy = this.isDummy(deviceObj); - this._devices[name] = { - name, - type, - device: deviceObj, - ip: addresses[0] || "", - active, - metered, - dummy - }; } // connect "state-changed" signals of new stored devices. @@ -350,8 +354,14 @@ export class DeviceMonitor { return addresses; } let ipConfig: NM.IPConfig | null = null; - if (family == GLib.SYSDEF_AF_INET) ipConfig = device.get_ip4_config(); - else ipConfig = device.get_ip6_config(); + try { + if (family == GLib.SYSDEF_AF_INET) ipConfig = device.get_ip4_config(); + else ipConfig = device.get_ip6_config(); + } catch (e) { + this._logger.info(`Failed to get IP config for device '${device.get_iface()}': ${e}`); + addresses[0] = "-"; + return addresses; + } if (ipConfig == null) { this._logger.info(`No config found for device '${device.get_iface()}'`); diff --git a/tsconfig.json b/tsconfig.json index 19b2e4c..043cd50 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,7 +6,8 @@ "sourceMap": false, "strict": true, "target": "ES2022", - "lib": ["ES2022"] + "lib": ["ES2022"], + "skipLibCheck": true }, "include": ["ambient.d.ts"], "files": ["extension.ts", "prefs.ts"]