Fix memory leak issues in window management by ptr7F · Pull Request #109 · flexagoon/rounded-window-corners

ptr7F · 2025-09-16T04:32:47Z

Complete fixes applied:

Fix signal disconnection in event_manager.ts by adding proper error handling and connection array cleanup
Add property binding tracking and cleanup in event_handlers.ts to prevent memory leaks
Improve shadow actor constraint cleanup to ensure all resources are properly released
Add comprehensive null checks throughout the codebase to prevent crashes
Enhance window creation handling with better error handling
Add proper cleanup logging for debugging
Fix window title handling to prevent null reference errors

This resolves VRAM continuously increasing after window close operations. The extension now properly manages the lifecycle of all window effects, property bindings, and shadow actors.

Complete fixes applied: - Fix signal disconnection in event_manager.ts by adding proper error handling and connection array cleanup - Add property binding tracking and cleanup in event_handlers.ts to prevent memory leaks - Improve shadow actor constraint cleanup to ensure all resources are properly released - Add comprehensive null checks throughout the codebase to prevent crashes - Enhance window creation handling with better error handling - Add proper cleanup logging for debugging - Fix window title handling to prevent null reference errors This resolves VRAM continuously increasing after window close operations. The extension now properly manages the lifecycle of all window effects, property bindings, and shadow actors. All modifications have been carefully verified to match the working version.

- Add null checks in getRoundedCornersEffect to prevent 'win is null' errors - Improve refreshRoundedCorners logic with better state handling and logging - Add texture signal disconnection to prevent GC callback issues - Fix TypeScript null safety issues in effect and windowInfo handling These fixes address the remaining JavaScript errors seen in the logs: - TypeError: win is null in getRoundedCornersEffect - GC callback issues with size-changed signal on MetaShapedTexture - Excessive repeated type checking and effect toggling

- Remove debounce mechanism that caused timing issues with window destruction - Add null checks in refreshShadow function to prevent 'win is null' errors - Simplify refreshRoundedCorners back to working version without excessive logging - Fix issues where disposed MetaWindowActor objects were still being accessed This addresses the regression introduced by the debounce optimization that caused g_closure_unref errors and null reference exceptions when windows were destroyed during the debounce timeout period.

flexagoon

Also, are you sure the VRAM usage is related to this? I've never noticed any VRAM spikes while using the extension

flexagoon · 2025-09-23T00:18:35Z

+    // Add null checks for safety
+    if (!actor || !actor.metaWindow) {
+        return;
+    }


Does this ever fail? If so, this should probably be handled somewhere earlier on a Typescript level

You're right. This was defensive programming on my part, but if actor or actor.metaWindow can actually be null/undefined, it should be handled at the TypeScript type level (making the parameters optional) rather than runtime checks. If they're never actually null in practice, these checks are unnecessary overhead.

flexagoon · 2025-09-23T00:20:17Z

+    // Store property bindings so we can clean them up later
+    const propertyBindings: GObject.Binding[] = [];
+


This is supposed to already be handled through the connections array in event_manager.ts. Is it getting reset for some reason?

The connections array in event_manager.ts only tracks signal connections (created with object.connect()), but actor.bind_property() creates GObject.Binding objects which are a different mechanism. Property bindings need to be explicitly unbound with binding.unbind() to prevent memory leaks - they can't be cleaned up through the signal disconnection system.

flexagoon · 2025-09-23T00:21:31Z

+    // Clean up property bindings
+    if (customData.propertyBindings) {
+        logDebug(`Cleaning up ${customData.propertyBindings.length} property bindings`);
+        for (const binding of customData.propertyBindings) {
+            try {
+                binding.unbind();
+            } catch (e) {
+                logDebug(`Failed to unbind property: ${e}`);
+            }
+        }
+    }


The removeEffectFrom function in event_manager should be handling this already

The removeEffectFrom function in event_manager.ts calls disconnectAll() which only handles signal disconnections (object.disconnect(id)). Property bindings created by bind_property() return GObject.Binding objects that must be cleaned up with binding.unbind() - they're not handled by the signal disconnection system. Without explicit unbinding, these create memory leaks through circular references between actor and shadow.

flexagoon · 2025-09-23T00:23:32Z

+    if (!hasEffect && shouldHaveEffect) {
+        logDebug(`Adding effect to ${win.title || 'unknown window'} (was missing)`);


As the comment here said, onAddEffect already skipped windows that shouldn't have rounded corners, why did you remove the comment and add an extra && shouldHaveEffect check here?

You're right. The extra && shouldHaveEffect check is redundant because onAddEffect already has the shouldEnableEffect(win) check at the beginning and returns early if the window shouldn't have the effect. This was an unnecessary defensive check on my part.

flexagoon · 2025-09-23T00:23:49Z

    }

-    if (!shouldHaveEffect) {
+    if (hasEffect && !shouldHaveEffect) {


This is already handled by the above if statement

You're right. The if (hasEffect && !shouldHaveEffect) condition at line 299 is redundant because it's already handled by the earlier logic flow. I mistakenly removed the original if (!effect.enabled) check which was actually needed to re-enable disabled effects. This was an unnecessary change that broke the original logic.

flexagoon · 2025-09-23T00:24:13Z


-    if (!effect.enabled) {
+    if (!hasEffect && !shouldHaveEffect) {
+        // Neither has nor should have effect - this is normal for some windows


Yes, and it was already handled by the top case

You're correct. That change was a logic error on my part. The original if (!effect.enabled) check was correct - it handles the case where the effect exists but is disabled and needs to be re-enabled. My change to if (!hasEffect && !shouldHaveEffect) doesn't make sense in that context and was already handled by the earlier conditions.

flexagoon · 2025-09-23T00:25:09Z

+    for (let i = 0; i < connections.length; i++) {
+        const connection = connections[i];


Why are you iterating the array like this instead of for (const connection of connections)?

for (const connection of connections) is cleaner and more readable. There's no reason to use the index-based loop here - it was just a coding habit on my part. The for...of syntax is more appropriate.

ptr7F · 2025-09-23T17:41:06Z

Also, are you sure the VRAM usage is related to this? I've never noticed any VRAM spikes while using the extension

Yes, I'm certain it's related. I initially thought it was a GNOME issue - I've been experiencing VRAM leaks since GNOME 47 through current GNOME 48, where VRAM usage keeps increasing and the only solution was logout/restart. I recently realized it might be an extension issue and systematically tested each one.

I could observe VRAM increasing each time I opened and closed windows with this extension enabled, but when I disabled the extension, opening/closing the same windows showed no VRAM leaks. I'm not experienced with TypeScript - I learned while having AI help me modify the code. After using the modified version for some time now, I haven't experienced any VRAM leaks.

You have a much better understanding of this project, so I'll follow your guidance on how to properly fix this.

The hardware I use is AMD CPU and graphics card.

flexagoon · 2025-11-08T14:15:22Z

@Y0J1G3N why did you close this?

ptr7F · 2025-11-08T14:28:09Z

I thought you weren’t going to merge it. Also, I created another new branch called "fix-memory-leak2", which is a further improvement. On my side, if I don’t apply this fix, the VRAM keeps leaking until it’s completely used up. However, if I disable this extension, the leak stops.

flexagoon · 2025-11-15T05:07:15Z

@Y0J1G3N can you please check the comment in #118 and see if that helps?

ptr7F added 3 commits September 16, 2025 11:57

flexagoon requested changes Sep 23, 2025

View reviewed changes

ptr7F closed this Nov 8, 2025

flexagoon mentioned this pull request Nov 11, 2025

VRAM memory leak #118

Closed

		// Store property bindings so we can clean them up later
		const propertyBindings: GObject.Binding[] = [];

		if (!hasEffect && shouldHaveEffect) {
		logDebug(`Adding effect to ${win.title \|\| 'unknown window'} (was missing)`);

		for (let i = 0; i < connections.length; i++) {
		const connection = connections[i];

Conversation

ptr7F commented Sep 16, 2025

Uh oh!

flexagoon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptr7F Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptr7F commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flexagoon commented Nov 8, 2025

Uh oh!

ptr7F commented Nov 8, 2025

Uh oh!

flexagoon commented Nov 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ptr7F Sep 23, 2025 •

edited

Loading

ptr7F commented Sep 23, 2025 •

edited

Loading