I've been holding off for some time on sending in this patch but since
there's talk of a release I thought I'd go ahead now. It fixes a SEGV
crasher as explained in the commit message. It's a sporadic bug, to
reproduce I flip back and forth between fullscreen mode and between
images and eventually it crashes, perhaps triggered by a race condition.
Lacking a deterministic test case means of course that it's hard to say
with 100% confidence that it's fixed but I'm quite certain since it
crashes very regularly for me without the fix.
I hadn't sent in the patch yet because I also have another fix for it,
which is slightly more complex and retains the existing read-ahead
image-loader on the new image object, but I have found no noticeable
difference in performance and as always, complexity is the enemy of
correctness and the friend of bloat, so I'm submitting the simpler
patch.
It's a nasty crasher, but perhaps not many others noticed it due to some
peculiarity of my setup, which amongst other things is dual-head/randr.
-- David
-- >8 --
Subject: Fix SEGV crasher (fullscreen + read-ahead)
When entering fullscreen, if not the "same region" as the "normal"
window, and the normal window had an active read-ahead image-loader,
crashed with a nasty SEGV when the read-ahead image-loader delivered its
'done' callback in image_read_ahead_done_cb() because imd->read_ahead_fd
was null pointer but deferenced.
Apparently no similar bug exists when entering fullscreen for "not same
region" ['copy' rather than 'move' the image object], the code for which
transfers the read-ahead image-loader to the new instance and cleans up
the references.
This patch simply cancels the read-ahead rather than transferring it for
the 'move' case, which is simpler and seems not to have any noticeable
adverse performance effects (from my testing).
Signed-off-by: David Favro <gee...@meta-dynamic.com>
---
src/image.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/image.c b/src/image.c
index 12fe8fe..8d4cf0e 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1195,6 +1195,14 @@ void image_move_from_image(ImageWindow *imd, ImageWindow
*source)
color_man_free((ColorMan *)imd->cm);
imd->cm = NULL;
+ /* Cancel the read-ahead: this avoids a race-condition crasher (in
+ * the "done" signal's callback) when an image is 'moved' with an
+ * active read-ahead image-loader.
+ */
+ if ( source->read_ahead_il ) {
+ image_read_ahead_cancel(source);
+ imd->read_ahead_il = NULL;
+ }
file_data_unref(imd->read_ahead_fd);
source->read_ahead_fd = NULL;
--
1.9.0.rc3
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk
_______________________________________________
Geeqie-devel mailing list
Geeqie-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geeqie-devel
