bug-gnu-emacs
[Top][All Lists]
Advanced

[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 <address@hidden> 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:

Attachment: 0001-Plug-memory-leak-in-GTK-x-display-monitor-attributes.patch
Description: v2


reply via email to

[Prev in Thread] Current Thread [Next in Thread]