Merge lp:~cmm2-deactivatedaccount/pantheon-files/memory-leak-fixes into lp:~elementary-apps/pantheon-files/trunk
- memory-leak-fixes
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jeremy Wootten | ||||
Approved revision: | 1790 | ||||
Merged at revision: | 1789 | ||||
Proposed branch: | lp:~cmm2-deactivatedaccount/pantheon-files/memory-leak-fixes | ||||
Merge into: | lp:~elementary-apps/pantheon-files/trunk | ||||
Diff against target: |
396 lines (+122/-30) 12 files modified
libcore/gof-directory-async.vala (+45/-11) libcore/gof-file.c (+10/-0) libcore/marlin-file-operations.c (+13/-9) libcore/pantheon-files-core-C.vapi (+1/-1) src/MimeActions.vala (+2/-2) src/TextRenderer.vala (+8/-1) src/View/AbstractDirectoryView.vala (+21/-1) src/View/AbstractTreeView.vala (+1/-1) src/View/IconView.vala (+1/-1) src/View/ListView.vala (+1/-1) src/View/PropertiesWindow.vala (+13/-2) src/marlin-icon-renderer.c (+6/-0) |
||||
To merge this branch: | bzr merge lp:~cmm2-deactivatedaccount/pantheon-files/memory-leak-fixes | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeremy Wootten | Approve | ||
Review via email: mp+254188@code.launchpad.net |
Commit message
Improve memory utilization and reduce memory leaks.
Description of the change
This branch attempts to fix most of the file related memory leaks in Pantheon Files.
The user interface patch is still somewhat experimental and may have side effects on unexpected areas of the program. It breaks some previous inadvertent assumptions that file objects are forever resident in memory. However, most of those issues should be fixed.
Memory utilization comparisons are listed below.
Preparation:
mkdir -p test/old test/new
pushd test/old
for i in $(seq 250000); do echo '' > $i; done
popd
./pantheon-
Procedure:
1) Measure memory usage.
2) Copy test/old to test/new/old.
3) Measure memory usage.
4) Create 10 files in test/new/ (this purges the undo manager history).
5) Delete test/new/*.
6) Repeat steps #1 through #5.
Results:
http://
Comparison:
http://
cmm2 (cmm2-deactivatedaccount) wrote : | # |
Jeremy Wootten (jeremywootten) wrote : | # |
Thanks for continuing to work on this cmm2! The branch substantially reduces memory usage on copy and deleting 250000 files as described. On my system, one cycle of the test described increased memory usage by over 500MB whereas in the branch it increased by about 75MB (Using System Monitor). For comparison, Thunar's memory increased by about 100MB. PCManFM however, used no extra memory (!) provided the files were not displayed. Presumably some information is still cached in Files and Thunar so this is not necessarily a leak?
No problems with the code.
You inserted a TODO - were you going to do this in this branch or later?
I'd like to test the branch for regressions but subject to that I will approve. Thanks again.
cmm2 (cmm2-deactivatedaccount) wrote : | # |
I need to look into it, but I suspect the initial 70-100M allocation is not actually used, but is simply freed memory that the allocator has not released to the OS. This explains why subsequent operations do not expand the total memory amount.
The blame for this additional memory requirement seems to fall on marlin_
Some file managers don't track destination files for undoing if they belong to a created subdirectory, thus making their aftermath memory requirement effectively nil.
> You inserted a TODO - were you going to do this in this branch or later?
Probably sometime later. If the selected_files problem was causing known crashes I'd fix it, but right now it seems like a pedantic hypothetical.
Jeremy Wootten (jeremywootten) wrote : | # |
I have not found any obvious regressions so I will approve this for merging now. Don't forget to claim your well-deserved bounty!
Preview Diff
1 | === modified file 'libcore/gof-directory-async.vala' | |||
2 | --- libcore/gof-directory-async.vala 2015-03-06 03:36:39 +0000 | |||
3 | +++ libcore/gof-directory-async.vala 2015-03-26 02:53:22 +0000 | |||
4 | @@ -48,7 +48,7 @@ | |||
5 | 48 | private Cancellable cancellable; | 48 | private Cancellable cancellable; |
6 | 49 | private FileMonitor? monitor = null; | 49 | private FileMonitor? monitor = null; |
7 | 50 | 50 | ||
9 | 51 | private List<GOF.File>? sorted_dirs = null; | 51 | private List<unowned GOF.File>? sorted_dirs = null; |
10 | 52 | 52 | ||
11 | 53 | public signal void file_loaded (GOF.File file); | 53 | public signal void file_loaded (GOF.File file); |
12 | 54 | public signal void file_added (GOF.File file); | 54 | public signal void file_added (GOF.File file); |
13 | @@ -466,6 +466,25 @@ | |||
14 | 466 | file_hash.insert (gof.location, gof); | 466 | file_hash.insert (gof.location, gof); |
15 | 467 | } | 467 | } |
16 | 468 | 468 | ||
17 | 469 | public GOF.File file_cache_find_or_insert (GLib.File file, | ||
18 | 470 | bool update_hash = false) | ||
19 | 471 | { | ||
20 | 472 | GOF.File? result = file_hash.lookup (file); | ||
21 | 473 | |||
22 | 474 | if (result == null) { | ||
23 | 475 | result = GOF.File.cache_lookup (file); | ||
24 | 476 | |||
25 | 477 | if (result == null) { | ||
26 | 478 | result = new GOF.File (file, location); | ||
27 | 479 | file_hash.insert (file, result); | ||
28 | 480 | } | ||
29 | 481 | else if (update_hash) | ||
30 | 482 | file_hash.insert (file, result); | ||
31 | 483 | } | ||
32 | 484 | |||
33 | 485 | return (!) result; | ||
34 | 486 | } | ||
35 | 487 | |||
36 | 469 | /* TODO move this to GOF.File */ | 488 | /* TODO move this to GOF.File */ |
37 | 470 | private delegate void func_query_info (GOF.File gof); | 489 | private delegate void func_query_info (GOF.File gof); |
38 | 471 | 490 | ||
39 | @@ -509,7 +528,9 @@ | |||
40 | 509 | 528 | ||
41 | 510 | if (!gof.is_hidden && gof.is_folder ()) { | 529 | if (!gof.is_hidden && gof.is_folder ()) { |
42 | 511 | /* add to sorted_dirs */ | 530 | /* add to sorted_dirs */ |
44 | 512 | sorted_dirs.insert_sorted (gof, GOF.File.compare_by_display_name); | 531 | if (sorted_dirs.find (gof) == null) |
45 | 532 | sorted_dirs.insert_sorted (gof, | ||
46 | 533 | GOF.File.compare_by_display_name); | ||
47 | 513 | } | 534 | } |
48 | 514 | 535 | ||
49 | 515 | if (track_longest_name && gof.basename.length > longest_file_name.length) { | 536 | if (track_longest_name && gof.basename.length > longest_file_name.length) { |
50 | @@ -523,7 +544,6 @@ | |||
51 | 523 | } | 544 | } |
52 | 524 | 545 | ||
53 | 525 | private void notify_file_added (GOF.File gof) { | 546 | private void notify_file_added (GOF.File gof) { |
54 | 526 | file_hash.insert (gof.location, gof); | ||
55 | 527 | query_info_async.begin (gof, add_and_refresh); | 547 | query_info_async.begin (gof, add_and_refresh); |
56 | 528 | } | 548 | } |
57 | 529 | 549 | ||
58 | @@ -533,6 +553,13 @@ | |||
59 | 533 | 553 | ||
60 | 534 | if (!gof.is_hidden && gof.is_folder ()) { | 554 | if (!gof.is_hidden && gof.is_folder ()) { |
61 | 535 | /* remove from sorted_dirs */ | 555 | /* remove from sorted_dirs */ |
62 | 556 | |||
63 | 557 | /* Addendum note: GLib.List.remove() does not unreference objects. | ||
64 | 558 | See: https://bugzilla.gnome.org/show_bug.cgi?id=624249 | ||
65 | 559 | https://bugzilla.gnome.org/show_bug.cgi?id=532268 | ||
66 | 560 | |||
67 | 561 | The declaration of sorted_dirs has been changed to contain | ||
68 | 562 | weak pointers as a temporary solution. */ | ||
69 | 536 | sorted_dirs.remove (gof); | 563 | sorted_dirs.remove (gof); |
70 | 537 | } | 564 | } |
71 | 538 | 565 | ||
72 | @@ -619,21 +646,23 @@ | |||
73 | 619 | 646 | ||
74 | 620 | public static void notify_files_changed (List<GLib.File> files) { | 647 | public static void notify_files_changed (List<GLib.File> files) { |
75 | 621 | foreach (var loc in files) { | 648 | foreach (var loc in files) { |
78 | 622 | GOF.File gof = GOF.File.get (loc); | 649 | Async? dir = cache_lookup_parent (loc); |
77 | 623 | Async? dir = cache_lookup (gof.directory); | ||
79 | 624 | 650 | ||
81 | 625 | if (dir != null) | 651 | if (dir != null) { |
82 | 652 | GOF.File gof = dir.file_cache_find_or_insert (loc); | ||
83 | 626 | dir.notify_file_changed (gof); | 653 | dir.notify_file_changed (gof); |
84 | 654 | } | ||
85 | 627 | } | 655 | } |
86 | 628 | } | 656 | } |
87 | 629 | 657 | ||
88 | 630 | public static void notify_files_added (List<GLib.File> files) { | 658 | public static void notify_files_added (List<GLib.File> files) { |
89 | 631 | foreach (var loc in files) { | 659 | foreach (var loc in files) { |
92 | 632 | GOF.File gof = GOF.File.get (loc); | 660 | Async? dir = cache_lookup_parent (loc); |
91 | 633 | Async? dir = cache_lookup (gof.directory); | ||
93 | 634 | 661 | ||
95 | 635 | if (dir != null) | 662 | if (dir != null) { |
96 | 663 | GOF.File gof = dir.file_cache_find_or_insert (loc, true); | ||
97 | 636 | dir.notify_file_added (gof); | 664 | dir.notify_file_added (gof); |
98 | 665 | } | ||
99 | 637 | } | 666 | } |
100 | 638 | } | 667 | } |
101 | 639 | 668 | ||
102 | @@ -642,10 +671,10 @@ | |||
103 | 642 | bool found; | 671 | bool found; |
104 | 643 | 672 | ||
105 | 644 | foreach (var loc in files) { | 673 | foreach (var loc in files) { |
108 | 645 | GOF.File gof = GOF.File.get (loc); | 674 | Async? dir = cache_lookup_parent (loc); |
107 | 646 | Async? dir = cache_lookup (gof.directory); | ||
109 | 647 | 675 | ||
110 | 648 | if (dir != null) { | 676 | if (dir != null) { |
111 | 677 | GOF.File gof = dir.file_cache_find_or_insert (loc); | ||
112 | 649 | dir.notify_file_removed (gof); | 678 | dir.notify_file_removed (gof); |
113 | 650 | found = false; | 679 | found = false; |
114 | 651 | 680 | ||
115 | @@ -725,6 +754,11 @@ | |||
116 | 725 | return cached_dir; | 754 | return cached_dir; |
117 | 726 | } | 755 | } |
118 | 727 | 756 | ||
119 | 757 | public static Async? cache_lookup_parent (GLib.File file) { | ||
120 | 758 | GLib.File? parent = file.get_parent (); | ||
121 | 759 | return parent != null ? cache_lookup (parent) : cache_lookup (file); | ||
122 | 760 | } | ||
123 | 761 | |||
124 | 728 | public bool remove_dir_from_cache () { | 762 | public bool remove_dir_from_cache () { |
125 | 729 | /* we got to increment the dir ref to remove the toggle_ref */ | 763 | /* we got to increment the dir ref to remove the toggle_ref */ |
126 | 730 | this.ref (); | 764 | this.ref (); |
127 | 731 | 765 | ||
128 | === modified file 'libcore/gof-file.c' | |||
129 | --- libcore/gof-file.c 2015-02-27 16:40:41 +0000 | |||
130 | +++ libcore/gof-file.c 2015-03-26 02:53:22 +0000 | |||
131 | @@ -114,6 +114,7 @@ | |||
132 | 114 | file->basename = g_file_get_basename (file->location); | 114 | file->basename = g_file_get_basename (file->location); |
133 | 115 | //file->parent_dir = g_file_enumerator_get_container (enumerator); | 115 | //file->parent_dir = g_file_enumerator_get_container (enumerator); |
134 | 116 | 116 | ||
135 | 117 | //g_debug ("%s: create %p", __func__, file); | ||
136 | 117 | return (file); | 118 | return (file); |
137 | 118 | } | 119 | } |
138 | 119 | 120 | ||
139 | @@ -946,6 +947,8 @@ | |||
140 | 946 | } | 947 | } |
141 | 947 | 948 | ||
142 | 948 | static void gof_file_finalize (GObject* obj) { | 949 | static void gof_file_finalize (GObject* obj) { |
143 | 950 | //g_debug ("%s: delete %p", __func__, obj); | ||
144 | 951 | |||
145 | 949 | GOFFile *file; | 952 | GOFFile *file; |
146 | 950 | 953 | ||
147 | 951 | file = GOF_FILE (obj); | 954 | file = GOF_FILE (obj); |
148 | @@ -977,6 +980,13 @@ | |||
149 | 977 | /* TODO remove the target_gof */ | 980 | /* TODO remove the target_gof */ |
150 | 978 | _g_free0 (file->thumbnail_path); | 981 | _g_free0 (file->thumbnail_path); |
151 | 979 | 982 | ||
152 | 983 | #ifndef NDEBUG | ||
153 | 984 | g_warn_if_fail (file->target_gof == NULL); | ||
154 | 985 | #endif | ||
155 | 986 | |||
156 | 987 | _g_free0 (file->owner); | ||
157 | 988 | _g_free0 (file->group); | ||
158 | 989 | |||
159 | 980 | G_OBJECT_CLASS (gof_file_parent_class)->finalize (obj); | 990 | G_OBJECT_CLASS (gof_file_parent_class)->finalize (obj); |
160 | 981 | } | 991 | } |
161 | 982 | 992 | ||
162 | 983 | 993 | ||
163 | === modified file 'libcore/marlin-file-operations.c' | |||
164 | --- libcore/marlin-file-operations.c 2015-03-01 10:56:59 +0000 | |||
165 | +++ libcore/marlin-file-operations.c 2015-03-26 02:53:22 +0000 | |||
166 | @@ -4002,19 +4002,23 @@ | |||
167 | 4002 | static gboolean | 4002 | static gboolean |
168 | 4003 | test_dir_is_parent (GFile *child, GFile *root) | 4003 | test_dir_is_parent (GFile *child, GFile *root) |
169 | 4004 | { | 4004 | { |
174 | 4005 | GFile *f; | 4005 | GFile *f = child; |
175 | 4006 | 4006 | GFile *prev = NULL; | |
176 | 4007 | f = g_file_dup (child); | 4007 | |
177 | 4008 | while (f) { | 4008 | if (g_file_equal (child, root)) |
178 | 4009 | return TRUE; | ||
179 | 4010 | |||
180 | 4011 | while ((f = g_file_get_parent (f))) { | ||
181 | 4012 | if (prev) g_object_unref (prev); | ||
182 | 4013 | |||
183 | 4009 | if (g_file_equal (f, root)) { | 4014 | if (g_file_equal (f, root)) { |
184 | 4010 | g_object_unref (f); | 4015 | g_object_unref (f); |
185 | 4011 | return TRUE; | 4016 | return TRUE; |
186 | 4012 | } | 4017 | } |
192 | 4013 | f = g_file_get_parent (f); | 4018 | prev = f; |
193 | 4014 | } | 4019 | } |
194 | 4015 | if (f) { | 4020 | if (prev) g_object_unref (prev); |
195 | 4016 | g_object_unref (f); | 4021 | |
191 | 4017 | } | ||
196 | 4018 | return FALSE; | 4022 | return FALSE; |
197 | 4019 | } | 4023 | } |
198 | 4020 | 4024 | ||
199 | 4021 | 4025 | ||
200 | === modified file 'libcore/pantheon-files-core-C.vapi' | |||
201 | --- libcore/pantheon-files-core-C.vapi 2015-03-01 10:56:59 +0000 | |||
202 | +++ libcore/pantheon-files-core-C.vapi 2015-03-26 02:53:22 +0000 | |||
203 | @@ -31,7 +31,7 @@ | |||
204 | 31 | public void add_file(GOF.File file, GOF.Directory.Async dir); | 31 | public void add_file(GOF.File file, GOF.Directory.Async dir); |
205 | 32 | public void remove_file (GOF.File file, GOF.Directory.Async dir); | 32 | public void remove_file (GOF.File file, GOF.Directory.Async dir); |
206 | 33 | public void file_changed (GOF.File file, GOF.Directory.Async dir); | 33 | public void file_changed (GOF.File file, GOF.Directory.Async dir); |
208 | 34 | public unowned GOF.File file_for_path(Gtk.TreePath path); | 34 | public GOF.File? file_for_path (Gtk.TreePath path); |
209 | 35 | public static GLib.Type get_type (); | 35 | public static GLib.Type get_type (); |
210 | 36 | public bool get_first_iter_for_file (GOF.File file, out Gtk.TreeIter iter); | 36 | public bool get_first_iter_for_file (GOF.File file, out Gtk.TreeIter iter); |
211 | 37 | public bool get_tree_iter_from_file (GOF.File file, GOF.Directory.Async directory, out Gtk.TreeIter iter); | 37 | public bool get_tree_iter_from_file (GOF.File file, GOF.Directory.Async directory, out Gtk.TreeIter iter); |
212 | 38 | 38 | ||
213 | === modified file 'src/MimeActions.vala' | |||
214 | --- src/MimeActions.vala 2015-02-06 22:57:55 +0000 | |||
215 | +++ src/MimeActions.vala 2015-03-26 02:53:22 +0000 | |||
216 | @@ -39,7 +39,7 @@ | |||
217 | 39 | public static AppInfo? get_default_application_for_files (GLib.List<unowned GOF.File> files) { | 39 | public static AppInfo? get_default_application_for_files (GLib.List<unowned GOF.File> files) { |
218 | 40 | assert (files != null); | 40 | assert (files != null); |
219 | 41 | /* Need to make a new list to avoid corrupting the selection */ | 41 | /* Need to make a new list to avoid corrupting the selection */ |
221 | 42 | unowned GLib.List<GOF.File> sorted_files = null; | 42 | GLib.List<unowned GOF.File> sorted_files = null; |
222 | 43 | files.@foreach ((file) => { | 43 | files.@foreach ((file) => { |
223 | 44 | sorted_files.prepend (file); | 44 | sorted_files.prepend (file); |
224 | 45 | }); | 45 | }); |
225 | @@ -120,7 +120,7 @@ | |||
226 | 120 | public static List<AppInfo>? get_applications_for_files (GLib.List<unowned GOF.File> files) { | 120 | public static List<AppInfo>? get_applications_for_files (GLib.List<unowned GOF.File> files) { |
227 | 121 | assert (files != null); | 121 | assert (files != null); |
228 | 122 | /* Need to make a new list to avoid corrupting the selection */ | 122 | /* Need to make a new list to avoid corrupting the selection */ |
230 | 123 | unowned GLib.List<GOF.File> sorted_files = null; | 123 | GLib.List<unowned GOF.File> sorted_files = null; |
231 | 124 | files.@foreach ((file) => { | 124 | files.@foreach ((file) => { |
232 | 125 | sorted_files.prepend (file); | 125 | sorted_files.prepend (file); |
233 | 126 | }); | 126 | }); |
234 | 127 | 127 | ||
235 | === modified file 'src/TextRenderer.vala' | |||
236 | --- src/TextRenderer.vala 2014-11-04 14:51:14 +0000 | |||
237 | +++ src/TextRenderer.vala 2015-03-26 02:53:22 +0000 | |||
238 | @@ -25,7 +25,7 @@ | |||
239 | 25 | public Marlin.ZoomLevel zoom_level {get; set;} | 25 | public Marlin.ZoomLevel zoom_level {get; set;} |
240 | 26 | public bool follow_state {get; set;} | 26 | public bool follow_state {get; set;} |
241 | 27 | public new string background { set; private get;} | 27 | public new string background { set; private get;} |
243 | 28 | public GOF.File file {set; private get;} | 28 | public GOF.File? file {set; private get;} |
244 | 29 | public int text_width; | 29 | public int text_width; |
245 | 30 | public int text_height; | 30 | public int text_height; |
246 | 31 | 31 | ||
247 | @@ -91,6 +91,11 @@ | |||
248 | 91 | layout); | 91 | layout); |
249 | 92 | 92 | ||
250 | 93 | style_context.restore (); | 93 | style_context.restore (); |
251 | 94 | |||
252 | 95 | /* The render call should always be preceded by a set_property call | ||
253 | 96 | from GTK. It should be safe to unreference or free the allocated | ||
254 | 97 | memory here. */ | ||
255 | 98 | file = null; | ||
256 | 94 | } | 99 | } |
257 | 95 | 100 | ||
258 | 96 | public void set_up_layout (string? text, Gdk.Rectangle cell_area) { | 101 | public void set_up_layout (string? text, Gdk.Rectangle cell_area) { |
259 | @@ -211,6 +216,7 @@ | |||
260 | 211 | 216 | ||
261 | 212 | private void invalidate () { | 217 | private void invalidate () { |
262 | 213 | set_widget (null); | 218 | set_widget (null); |
263 | 219 | file = null; | ||
264 | 214 | } | 220 | } |
265 | 215 | 221 | ||
266 | 216 | private void on_entry_editing_done () { | 222 | private void on_entry_editing_done () { |
267 | @@ -224,6 +230,7 @@ | |||
268 | 224 | string path = entry.get_data ("marlin-text-renderer-path"); | 230 | string path = entry.get_data ("marlin-text-renderer-path"); |
269 | 225 | edited (path, text); | 231 | edited (path, text); |
270 | 226 | } | 232 | } |
271 | 233 | file = null; | ||
272 | 227 | } | 234 | } |
273 | 228 | 235 | ||
274 | 229 | private bool on_entry_focus_out_event (Gdk.Event event) { | 236 | private bool on_entry_focus_out_event (Gdk.Event event) { |
275 | 230 | 237 | ||
276 | === modified file 'src/View/AbstractDirectoryView.vala' | |||
277 | --- src/View/AbstractDirectoryView.vala 2015-03-25 08:59:04 +0000 | |||
278 | +++ src/View/AbstractDirectoryView.vala 2015-03-26 02:53:22 +0000 | |||
279 | @@ -192,7 +192,27 @@ | |||
280 | 192 | 192 | ||
281 | 193 | private GLib.List<GLib.AppInfo> open_with_apps; | 193 | private GLib.List<GLib.AppInfo> open_with_apps; |
282 | 194 | protected GLib.List<GOF.Directory.Async>? loaded_subdirectories = null; | 194 | protected GLib.List<GOF.Directory.Async>? loaded_subdirectories = null; |
284 | 195 | protected GLib.List<unowned GOF.File> selected_files = null ; | 195 | |
285 | 196 | /* TODO: Remove the "unowned" portion of the declaration for | ||
286 | 197 | selected_files and on code that repeats its type. | ||
287 | 198 | |||
288 | 199 | Selected files are originally obtained with | ||
289 | 200 | gtk_tree_model_get(): this function increases the reference | ||
290 | 201 | count of the file object. | ||
291 | 202 | |||
292 | 203 | In order to prevent the obvious memory leak when inserting | ||
293 | 204 | these owned objects into the unowned element container, the | ||
294 | 205 | objects are unreferenced upon being inserted. | ||
295 | 206 | |||
296 | 207 | This results in a container filled with weak references to | ||
297 | 208 | objects with reference counts of 1. | ||
298 | 209 | |||
299 | 210 | A scenario may occur that the cell holding the original file | ||
300 | 211 | object is destroyed, and the container is left with pointers | ||
301 | 212 | pointing to freed memory. Reference counting the container | ||
302 | 213 | elements would prevent this possibility. */ | ||
303 | 214 | protected GLib.List<unowned GOF.File> selected_files = null; | ||
304 | 215 | |||
305 | 196 | private GLib.List<unowned GOF.File>? templates = null; | 216 | private GLib.List<unowned GOF.File>? templates = null; |
306 | 197 | 217 | ||
307 | 198 | private GLib.AppInfo default_app; | 218 | private GLib.AppInfo default_app; |
308 | 199 | 219 | ||
309 | === modified file 'src/View/AbstractTreeView.vala' | |||
310 | --- src/View/AbstractTreeView.vala 2015-02-12 18:28:34 +0000 | |||
311 | +++ src/View/AbstractTreeView.vala 2015-03-26 02:53:22 +0000 | |||
312 | @@ -163,7 +163,7 @@ | |||
313 | 163 | selected_files = null; | 163 | selected_files = null; |
314 | 164 | 164 | ||
315 | 165 | tree.get_selection ().selected_foreach ((model, path, iter) => { | 165 | tree.get_selection ().selected_foreach ((model, path, iter) => { |
317 | 166 | unowned GOF.File? file; /* can be null if click on blank row in list view */ | 166 | GOF.File? file; /* can be null if click on blank row in list view */ |
318 | 167 | model.@get (iter, FM.ListModel.ColumnID.FILE_COLUMN, out file, -1); | 167 | model.@get (iter, FM.ListModel.ColumnID.FILE_COLUMN, out file, -1); |
319 | 168 | if (file != null) | 168 | if (file != null) |
320 | 169 | selected_files.prepend (file); | 169 | selected_files.prepend (file); |
321 | 170 | 170 | ||
322 | === modified file 'src/View/IconView.vala' | |||
323 | --- src/View/IconView.vala 2015-02-12 18:28:34 +0000 | |||
324 | +++ src/View/IconView.vala 2015-03-26 02:53:22 +0000 | |||
325 | @@ -172,7 +172,7 @@ | |||
326 | 172 | selected_files = null; | 172 | selected_files = null; |
327 | 173 | 173 | ||
328 | 174 | tree.selected_foreach ((tree, path) => { | 174 | tree.selected_foreach ((tree, path) => { |
330 | 175 | unowned GOF.File file; | 175 | GOF.File? file; |
331 | 176 | file = model.file_for_path (path); | 176 | file = model.file_for_path (path); |
332 | 177 | 177 | ||
333 | 178 | if (file != null) | 178 | if (file != null) |
334 | 179 | 179 | ||
335 | === modified file 'src/View/ListView.vala' | |||
336 | --- src/View/ListView.vala 2015-01-24 12:31:20 +0000 | |||
337 | +++ src/View/ListView.vala 2015-03-26 02:53:22 +0000 | |||
338 | @@ -89,7 +89,7 @@ | |||
339 | 89 | } | 89 | } |
340 | 90 | 90 | ||
341 | 91 | private void set_path_expanded (Gtk.TreePath path, bool expanded) { | 91 | private void set_path_expanded (Gtk.TreePath path, bool expanded) { |
343 | 92 | unowned GOF.File? file = model.file_for_path (path); | 92 | GOF.File? file = model.file_for_path (path); |
344 | 93 | 93 | ||
345 | 94 | if (file != null) | 94 | if (file != null) |
346 | 95 | file.set_expanded (expanded); | 95 | file.set_expanded (expanded); |
347 | 96 | 96 | ||
348 | === modified file 'src/View/PropertiesWindow.vala' | |||
349 | --- src/View/PropertiesWindow.vala 2015-03-06 03:36:39 +0000 | |||
350 | +++ src/View/PropertiesWindow.vala 2015-03-26 02:53:22 +0000 | |||
351 | @@ -40,7 +40,7 @@ | |||
352 | 40 | 40 | ||
353 | 41 | private uint count; | 41 | private uint count; |
354 | 42 | private GLib.List<GOF.File> files; | 42 | private GLib.List<GOF.File> files; |
356 | 43 | private GOF.File goffile; | 43 | private unowned GOF.File goffile; |
357 | 44 | private FM.AbstractDirectoryView view; | 44 | private FM.AbstractDirectoryView view; |
358 | 45 | 45 | ||
359 | 46 | private Gee.Set<string>? mimes; | 46 | private Gee.Set<string>? mimes; |
360 | @@ -105,7 +105,18 @@ | |||
361 | 105 | } | 105 | } |
362 | 106 | 106 | ||
363 | 107 | view = _view; | 107 | view = _view; |
365 | 108 | files = _files.copy (); | 108 | |
366 | 109 | /* The properties window may outlive the passed-in file object | ||
367 | 110 | lifetimes. The objects must be referenced as a precaution. | ||
368 | 111 | |||
369 | 112 | GLib.List.copy() would not guarantee valid references: because it | ||
370 | 113 | does a shallow copy (copying the pointer values only) the objects' | ||
371 | 114 | memory may be freed even while this code is using it. */ | ||
372 | 115 | foreach (unowned GOF.File file in _files) | ||
373 | 116 | /* prepend(G) is declared "owned G", so ref() will be called once | ||
374 | 117 | on the unowned foreach value. */ | ||
375 | 118 | files.prepend (file); | ||
376 | 119 | |||
377 | 109 | count = files.length(); | 120 | count = files.length(); |
378 | 110 | 121 | ||
379 | 111 | if (count < 1 ) { | 122 | if (count < 1 ) { |
380 | 112 | 123 | ||
381 | === modified file 'src/marlin-icon-renderer.c' | |||
382 | --- src/marlin-icon-renderer.c 2015-01-18 20:46:03 +0000 | |||
383 | +++ src/marlin-icon-renderer.c 2015-03-26 02:53:22 +0000 | |||
384 | @@ -661,6 +661,12 @@ | |||
385 | 661 | g_object_unref (pix); | 661 | g_object_unref (pix); |
386 | 662 | } | 662 | } |
387 | 663 | } | 663 | } |
388 | 664 | |||
389 | 665 | /* The render call should always be preceded by a set_property call from | ||
390 | 666 | GTK. It should be safe to unreference or free the allocate memory | ||
391 | 667 | here. */ | ||
392 | 668 | _g_object_unref0 (priv->file); | ||
393 | 669 | _g_object_unref0 (priv->drop_file); | ||
394 | 664 | } | 670 | } |
395 | 665 | 671 | ||
396 | 666 | /* | 672 | /* |
My apologies for the delay in submitting these changes. I forgot about this for a while, until Jeremy removed me from the bug listing. I should've mentioned something earlier.
Shouldn't be anything too unusual with the patches. Of course, let me know if something goes wrong.