Commit Graph

14 Commits

Author SHA1 Message Date
Sebastian Keller
91ce5ca960 signalTracker: Remove SignalTracker after its last signal got untracked
The object the SignalTracker belongs to is stored in a map managed by
the SignalManager which keeps a reference to that object. This map is
never destroyed nor is any entry ever removed. This leads to all objects
that ever had SignalTrackers used on them being kept alive even after
all references outside of the SignalTracker are long gone. This then
also extends to other objects which are leaked indirectly through
reference chains from these objects.

And if some of those objects are GObjects, this will prevent them from
being finalized, leaking further resources. A StWidget for example will
not release its shadow textures.

Fix this by destroying the SignalTracker and removing it from the
SignalManager once the last signal it was tracking has been untracked.

A WeakMap could have been used as well, but we need the Map to be
iterable in some of the following changes.

Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5807
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5796
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2466>
2022-09-03 15:11:46 +00:00
Sebastian Keller
54ee728aa0 signalTracker: Don't try to create a new SignalTracker for disconnecting
After the next commit, when some classes, such as PopupMenuManager try
to disconnect via a destroy handler, the SignalTracker might have
already been destroyed, so trying to get it from the SignalManager will
cause it to create a new one, which will then try to connect to the
destroy signal of the already destroyed object.

This could for example be triggered by changing backgrounds.

Fix this by not doing anything in disconnectObject if there is no
SignalTracker for that object.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2466>
2022-09-03 15:11:46 +00:00
Sebastian Keller
d57953ad94 Revert "signalTracker: Use WeakMap in SignalManager to avoid leaking objects"
This reverts commit 085102be74.

We need the SignalManager map top be iterable at shutdown for some of
the following changes. A WeakMap is not iterable. This revert changes it
back to a regular Map, which re-introduces the leaks caused by this.
Those will be fixed differently by the two followup commits.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2466>
2022-09-03 15:11:46 +00:00
Sebastian Keller
085102be74 signalTracker: Use WeakMap in SignalManager to avoid leaking objects
The object the SignalTracker belongs to is stored in a map managed by
the SignalManager which keeps a reference to that object. This map is
never destroyed nor is any entry ever removed. This leads to all objects
that ever had SignalTrackers used on them being kept alive even after
all references outside of the SignalTracker are long gone. This then
also extends to other objects which are leaked indirectly through
reference chains from these objects.

And if some of those objects are GObjects, this will prevent them from
being finalized, leaking further resources. A StWidget for example will
not release its shadow textures.

Fix this by using a WeakMap in SignalManager.

Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5807
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5796
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2450>
2022-08-29 21:53:58 +00:00
Marco Trevisan (Treviño)
9abf3508fd signalTracker: Use a const variable to hold the tracked object value
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2366>
2022-07-06 18:24:41 +02:00
Marco Trevisan (Treviño)
371da8d394 signalTracker: Correctly guard against invalid GObject.ConnectFlags
We considered any ConnectFlag value major than SWAPPED as invalid, while
it's technically not fully true as we need to ensure that the passed
value is respecting the whole flags mask.
In fact, per se SWAPPED|AFTER (> SWAPPED) is a valid value (even if we
don't support the AFTER value).

But this makes the check more future-proof.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2366>
2022-07-06 18:24:15 +02:00
Marco Trevisan (Treviño)
ad0f11f024 signalTracker: Avoid getting the same owner object proto multiple times
While untracking an object we used to compute it's proto for each signal
we were disconnecting from, while this is not needed when we're just
iterating over all the same owner signals, so let's add few more
functions to compute an object prototype, and repeat the disconnections
in the simplest way we can.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2366>
2022-07-06 18:24:12 +02:00
Marco Trevisan (Treviño)
74720f250e signalTracker: Avoid creating a temporary keys array when clearing
We used to create a temporary array of signal tracker keys and then to
iterate through them in order to untrack the objects, but the Map's can
be iterated directly so let's just use their native forEach.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2366>
2022-07-06 18:24:08 +02:00
Marco Trevisan (Treviño)
68ceed5c28 signalTracker: Avoid doing two maps lookups once they're initialized
Avoid checking if a map contains a value, given that in the case it's
not there, get() will just return an undefined value. And only in such
case we need to set the value.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2366>
2022-07-06 18:24:04 +02:00
Evan Welsh
a88e59c1a8 Adopt EventEmitter class instead of injecting Signal methods
Introduce a new class, EventEmitter, which implements signal
handling for pure JavaScript classes. EventEmitter still
utilizes GJS' addSignalMethods internally.

EventEmitter allows static typechecking to understand the
structure of event-emitting JS classes and makes creating
child classes simpler.

The name 'EventEmitter' mirrors a common name for this pattern
in Node and in JS libraries.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2043>
2022-07-04 18:30:49 -04:00
Florian Müllner
fc4f9f61fa signalTracker: Explicitly register destroyable types
We currently assume that any '::destroy' signal on a GObject type
has the semantics of the ClutterActor/GtkWidget signal, and should
therefore result in all signals being disconnected.

But we already have a case where the assumption doesn't hold: ShellWM
uses '::destroy' for the closing animation of windows, and the ShellWM
object itself remains very valid after the emission.

So rather than making assumptions about '::destroy', check objects
against a list of destroyable types that are explicitly registered
as such.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2226>
2022-03-06 00:10:03 +00:00
Florian Müllner
cf29ec2f22 signalTracker: Add TransientSignalHolder class
There are cases where we want to connect to a number of signals
for the lifetime of an object, but also other signals for a
limited period (say: between show and hide).

It is currently not possible to use disconnectObject() for the
latter, because it will disconnect all signals.

To address this use case, add a small class that can be used as
a transient signal holder, while still benefiting from autocleanup
by proxying the real owner.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2221>
2022-03-05 00:12:27 +00:00
Florian Müllner
7b0a94b246 signalTracker: Use connect_after to track 'destroy'
The 'destroy' signal currently doesn't work with connectObject(),
because the handler is only connected after the signal tracker's
own destroy handler, which disconnects all handlers.

Address this by using connect_after for the cleanup handler, so
that other destroy handlers run before it (unless they also use
ConnectFlags.AFTER, but well *shrug*).

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2221>
2022-03-05 00:12:27 +00:00
Florian Müllner
f45ccc9143 signalTracker: Provide monkey-patching for (dis)connectObject()
The module exports a `addObjectSignalMethods()` method that extends
the provided prototype with `connectObject()` and `disconnectObject()`
methods.

In its simplest form, `connectObject()` looks like the regular
`connect()` method, except for an additional parameter:

```js
    this._button.connectObject('clicked',
        () => this._onButtonClicked(), this);
```

The additional object can be used to disconnect all handlers on the
instance that were connected with that object, similar to
`g_signal_handlers_disconnect_by_data()` (which cannot be used
from introspection).

For objects that are subclasses of Clutter.Actor, that will happen
automatically when the actor is destroyed, similar to
`g_signal_connect_object()`.

Finally, `connectObject()` allows to conveniently connect multiple
signals at once, similar to `g_object_connect()`:

```js
    this._toggleButton.connect(
        'clicked', () => this._onClicked(),
        'notify::checked', () => this._onChecked(), this);
```

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1953>
2022-03-04 14:14:37 +00:00