[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#35179: [PATCH] Plug memory leak in GTK x-display-monitor-attributes-
From: |
Alex |
Subject: |
bug#35179: [PATCH] Plug memory leak in GTK x-display-monitor-attributes-list |
Date: |
Sun, 07 Apr 2019 11:34:28 -0600 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Eli Zaretskii <eliz@gnu.org> writes:
>> The main memory leak, though, was that the MonitorList array wasn't
>> being freed.
>
> Does your patch change that? If not, why not?
It did, yes. That was the xfree (monitors) call.
>> I considered using the free_monitors procedure like the non-GTK
>> versions do, but I saw no reason to call g_strdup for each name and
>> free each name almost right after.
>
> I don't see how the short lifetime of the array changes anything
> here. As long as we aren't sure the pointer returned by
> gdk_monitor_get_model is a copy that cannot be changed by another
> thread, we should ourselves make a copy. Otherwise, who can ensure us
> that some other GTK thread doesn't call this same function during the
> short life time of the array?
The documentation does state that the name property of the monitor is
read-only.
>> Since make_monitor_attribute_list uses make_string on each name, I don't
>> see any issues that this removal would cause.
>
> The issue I see is that between the time we call gdk_monitor_get_model
> and the time we use the string something could change the string to
> which the pointer points. If you can spot something in the GDK docs
> that guarantees this couldn't happen, please point me to that piece of
> docs.
Well, I suppose that the monitor could be unplugged in-between, which
presumably would mean that the monitor object is freed. Would that space
be reused in-between, though?
I suppose it doesn't hurt to play it safe, so I updated the patch to use
free_monitors instead:
0001-Plug-memory-leak-in-GTK-x-display-monitor-attributes.patch
Description: v2