Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backend: dummy: do not leak owned pixmaps #1022

Conversation

absolutelynothelix
Copy link
Collaborator

@absolutelynothelix absolutelynothelix commented Feb 13, 2023

fix a typo and an issue reported by xrescheck:

$ xrescheck.sh -p resource_leaked -r -t xcb_composite_named_windows_pixmaps,xcb_pixmaps ~/Work/picom/build/src/picom --backend=dummy
? xrescheck 0.0.1 is here!
[ 06/13/2023 00:05:55.378 session_init INFO ] Switching to log file: /home/helix/.local/state/picom.log
? checking for leaked resources...
! 0x3200027 allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)
! 0x320002b allocated by xcb_create_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/x.c:599
	/home/helix/Work/picom/build/../src/backend/backend_common.c:212
	/home/helix/Work/picom/build/../src/backend/backend_common.c:302
! 0x320002f allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)
! 0x3200033 allocated by xcb_create_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/x.c:599
	/home/helix/Work/picom/build/../src/backend/backend_common.c:212
	/home/helix/Work/picom/build/../src/backend/backend_common.c:302
! 0x3200037 allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)
! 0x320003b allocated by xcb_create_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/x.c:599
	/home/helix/Work/picom/build/../src/backend/backend_common.c:212
	/home/helix/Work/picom/build/../src/backend/backend_common.c:302
! 0x320003f allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)
! 0x3200043 allocated by xcb_create_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/x.c:599
	/home/helix/Work/picom/build/../src/backend/backend_common.c:212
	/home/helix/Work/picom/build/../src/backend/backend_common.c:302
! 0x3200047 allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)
! 0x320004b allocated by xcb_create_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/x.c:599
	/home/helix/Work/picom/build/../src/backend/backend_common.c:212
	/home/helix/Work/picom/build/../src/backend/backend_common.c:302
! 0x320004f allocated by xcb_composite_name_window_pixmap_checked wasn't freed, the allocation was made here:
	/home/helix/Work/picom/build/../src/win.c:326
	/home/helix/Work/picom/build/../src/win.c:573
	/home/helix/Work/picom/build/../src/picom.c:1670 (discriminator 2)

@codecov
Copy link

codecov bot commented Feb 13, 2023

Codecov Report

Merging #1022 (223872b) into next (5b6f6ec) will increase coverage by 0.01%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1022      +/-   ##
==========================================
+ Coverage   37.58%   37.59%   +0.01%     
==========================================
  Files          49       49              
  Lines       11155    11160       +5     
==========================================
+ Hits         4193     4196       +3     
- Misses       6962     6964       +2     
Impacted Files Coverage Δ
src/backend/dummy/dummy.c 65.78% <60.00%> (-0.41%) ⬇️

@absolutelynothelix
Copy link
Collaborator Author

absolutelynothelix commented Mar 4, 2023

it works kinda weird with the root back pixmap and i can't figure out why 🤔

@yshui
Copy link
Owner

yshui commented Mar 30, 2023

it works kinda weird with the root back pixmap

how weird?

@absolutelynothelix
Copy link
Collaborator Author

absolutelynothelix commented Apr 2, 2023

how weird?

iirc,

  1. do watch -n1 xprop -root _XROOTPMAP_ID;
  2. start and stop picom with backend other than dummy a few times and observe that the root back pixmap always binds successfully and it's id changes;
  3. start picom with dummy backend once and observe that the root back pixmap binds successfully but it's id lefts unchanged;
  4. start picom with dummy backend a few times and observe that the root back pixmap always fails to bind and it's id lefts unchanged;
  5. go back to step 2.

do you have any ideas what could cause such behavior? i didn't have much time to investigate when i encountered this issue.

btw, i'm alive, just a bit short on time. i should be able to dedicate a lot of time to picom soon 🙂

@absolutelynothelix
Copy link
Collaborator Author

absolutelynothelix commented Apr 23, 2023

i have an idea! looks like it's about pixmap ownership controlled by the owned parameter when binding one.

@absolutelynothelix absolutelynothelix force-pushed the xrescheck-xcb-composite-named-windows-pixmaps-test branch from 4e243c3 to f973278 Compare April 23, 2023 13:39
@absolutelynothelix
Copy link
Collaborator Author

absolutelynothelix commented Apr 23, 2023

i have an idea! looks like it's about pixmap ownership controlled by the owned parameter when binding one.

exactly. now the fix is complete.

free pixmaps which ownership was transferred to the backend
@absolutelynothelix absolutelynothelix force-pushed the xrescheck-xcb-composite-named-windows-pixmaps-test branch from f973278 to 223872b Compare June 12, 2023 21:02
@absolutelynothelix absolutelynothelix changed the title backend: dummy: fix images' pixmaps leak backend: dummy: do not leak owned pixmaps Jun 12, 2023
@yshui yshui merged commit 037fd4c into yshui:next Jun 13, 2023
6 checks passed
@absolutelynothelix absolutelynothelix deleted the xrescheck-xcb-composite-named-windows-pixmaps-test branch June 13, 2023 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants