After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 568760 - nautilus freezes due to a bug in garray.c:322
nautilus freezes due to a bug in garray.c:322
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal major
: ---
Assigned To: Sven Herzberg
gtkdev
Depends on:
Blocks:
 
 
Reported: 2009-01-22 20:48 UTC by Ian Blanes
Modified: 2010-04-20 13:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sanity check added (518 bytes, patch)
2009-01-22 21:12 UTC, Ian Blanes
none Details | Review
Properly tested patch (2.95 KB, patch)
2010-01-13 14:02 UTC, Sven Herzberg
committed Details | Review
Patch fixing the test case (950 bytes, patch)
2010-01-26 14:31 UTC, Sven Herzberg
needs-work Details | Review
patch fixing the test case v2 (477 bytes, patch)
2010-02-03 12:18 UTC, Sven Herzberg
none Details | Review

Description Ian Blanes 2009-01-22 20:48:36 UTC
Under certain conditions garray.c:g_nearest_pow may enter an infinite loop.

316 	static gint
317 	g_nearest_pow (gint num)
318 	{
319 	gint n = 1;
320 	
321 	while (n < num)
322 	n <<= 1;
323 	
324 	return n;
325 	} 

when called with log_2(num) > 30 the while condition is never meet, thus producing an infinite loop.

--

Program received signal SIGINT, Interrupt.

Thread 3064772384 (LWP 6554)

  • #0 g_nearest_pow
    at /build/buildd/glib2.0-2.16.6/glib/garray.c line 322
  • #1 g_array_maybe_expand
    at /build/buildd/glib2.0-2.16.6/glib/garray.c line 336
  • #2 IA__g_array_set_size
    at /build/buildd/glib2.0-2.16.6/glib/garray.c line 195
  • #3 IA__g_byte_array_set_size
    at /build/buildd/glib2.0-2.16.6/glib/garray.c line 670
  • #4 load_contents_read_callback
    at /build/buildd/glib2.0-2.16.6/gio/gfile.c line 5104
  • #5 async_ready_callback_wrapper
    at /build/buildd/glib2.0-2.16.6/gio/ginputstream.c line 473
  • #6 IA__g_simple_async_result_complete
    at /build/buildd/glib2.0-2.16.6/gio/gsimpleasyncresult.c line 553
  • #7 complete_in_idle_cb
    at /build/buildd/glib2.0-2.16.6/gio/gsimpleasyncresult.c line 563
  • #8 g_idle_dispatch
    at /build/buildd/glib2.0-2.16.6/glib/gmain.c line 4090
  • #9 IA__g_main_context_dispatch
    at /build/buildd/glib2.0-2.16.6/glib/gmain.c line 2012
  • #10 g_main_context_iterate
    at /build/buildd/glib2.0-2.16.6/glib/gmain.c line 2645
  • #11 IA__g_main_loop_run
    at /build/buildd/glib2.0-2.16.6/glib/gmain.c line 2853
  • #12 gtk_main
    from /usr/lib/libgtk-x11-2.0.so.0
  • #13 main
    at nautilus-main.c line 569

Comment 1 Ian Blanes 2009-01-22 21:12:35 UTC
Created attachment 127038 [details] [review]
sanity check added
Comment 2 Sven Herzberg 2010-01-13 14:02:24 UTC
Created attachment 151338 [details] [review]
Properly tested patch

This patch adds the check to the loop - and also in a more maintainble shape (read: easier to understand for people unfamiliar with this bug).

It also adds a testcase for the bug, so we're unlikely to introduce that bug again.
Comment 3 Matthias Clasen 2010-01-13 14:29:09 UTC
Looks good, please commit to the stable branch as well.
Comment 5 Matthias Clasen 2010-01-25 02:55:40 UTC
Hey Sven,

unfortunately, I am seeing your test fail (on x86_64):

GLib-ERROR **: gmem.c:175: failed to allocate 2147483648 bytes
aborting...
**
ERROR:array-test.c:118:array_large_size: child process (14390) of test trap failed unexpectedly
Aborted (core dumped)
Comment 6 Sven Herzberg 2010-01-26 14:24:21 UTC
(In reply to comment #5)
> unfortunately, I am seeing your test fail (on x86_64):
> 
> GLib-ERROR **: gmem.c:175: failed to allocate 2147483648 bytes
> aborting...
> **
> ERROR:array-test.c:118:array_large_size: child process (14390) of test trap
> failed unexpectedly
> Aborted (core dumped)

Too bad. Is there a way to inject a custom allocation function into glib? Allocating 2GB of memory isn't that useful indeed. I'll check if I can reduce the test to something less memory-extensive (like using char in GArray instead of gpointer via GPtrArray -- this should reduce the allocation from 2GB to 256MB at least).
Comment 7 Sven Herzberg 2010-01-26 14:31:28 UTC
Created attachment 152311 [details] [review]
Patch fixing the test case

Here's a patch that should let the test pass, even if the memory allocation failed.

Can you please test this patch, Matthias?
Comment 8 Matthias Clasen 2010-01-26 16:23:57 UTC
Yes, you can use g_mem_set_vtable to replace the malloc implementation. 

I'll test your patch anyway, thanks.
Comment 9 Sven Herzberg 2010-02-02 15:21:16 UTC
(In reply to comment #8)
> Yes, you can use g_mem_set_vtable to replace the malloc implementation. 
> 
> I'll test your patch anyway, thanks.

Any results?
Comment 10 Matthias Clasen 2010-02-02 17:27:08 UTC
nah, forgotten. sorry
added to todo list now
Comment 11 Matthias Clasen 2010-02-03 04:59:26 UTC
doesn't build:

array-test.c: In function ‘array_large_size’:
array-test.c:118: error: void value not ignored as it ought to be
make: *** [array-test.o] Error 1
Comment 12 Sven Herzberg 2010-02-03 12:18:08 UTC
Created attachment 152921 [details] [review]
patch fixing the test case v2

Narf, sorry. This one should compile.
Comment 13 Matthias Clasen 2010-02-21 21:31:58 UTC
Thanks, this works.
Comment 14 Emilio Pozuelo Monfort 2010-04-20 13:32:44 UTC
The test fails for me because the timeout is too low. I suspect bug 610784 and bug 613060 fail because of that too. Maybe they should be marked as dups

Can we make the timeout higher? After all if the test finishes before it won't keep waiting but continue.

Also making it not allocate 2GB of memory as comment 6 and 8 suggest would be nice.
Comment 15 Sven Herzberg 2010-04-20 13:41:49 UTC
(In reply to comment #14)
> The test fails for me because the timeout is too low. I suspect bug 610784 and
> bug 613060 fail because of that too. Maybe they should be marked as dups
> 
> Can we make the timeout higher? After all if the test finishes before it won't
> keep waiting but continue.
> 
> Also making it not allocate 2GB of memory as comment 6 and 8 suggest would be
> nice.

This works for me. Feel free to use the information from comment 6 and comment 8 to provide patches that run faster and that don't require as much memory…