Bug#996805: nemo: Freezes when opening a folder that contains a large GIF file
Hello,
I decided to take another shot at this and I still can't believe it, but I actually ended up fixing it.
Hooking a debugger to Nemo and stopping it in the middle of the freeze shows that it's hanging inside libgdk_pixbuf-2.0.so. More specifically, once Nemo calls gdk_pixbuf_loader_write() inside get_pixbuf_for_content() (in nemo-directory-async.c) the function takes a very long time to return or never returns at all for very large GIFs. Most likely, this means there's a regression inside libgdk_pixbuf-2.0.so, which also makes sense in the context of my original report: the bug is present in Debian 11 and Arch (both use version 2.42), but not in Debian 10 (which uses version 2.38) nor in Mint 20.1 (which uses version 2.40). The code of get_pixbuf_for_content() between Debian and Mint's source packages is also the same, and doesn't change for Nemo 5.0.3 either, so the whole picture is congruent. I tried "forward-porting" Buster's libgdk_pixbuf just for confirmation, but unfortunately that doesn't seem compatible with the rest of the system.
When looking at get_pixbuf_for_content(), there's a comment mentioning the need to call gdk_pixbuf_loader_write() with chunks of the file to process, however Nemo's implementation doesn't actually do this and shoves the whole file in one go at gdk_pixbuf_loader_write() which then seems to choke on it. Making slight adjustments so that the file is actually served in chunks as the comment mentions actually fixes the whole issue. I recompiled Nemo with such changes and the fix seems to hold.
Nemo's original get_pixbuf_for_content() is pretty self-contained so it can be extracted into a standalone program of just a handful of lines as a minimal reproducible example. I did it and tried it both on Debian 11 and Mint 20.1 and the behavior is as expected: it freezes on Debian but works on Mint. I also tested the changes in this way to take time measurements for different chunk sizes and compare performance between the fixed version and the original one on Mint.
I'm attaching a patch for Nemo's get_pixbuf_for_content() as well as the examples mentioned (which aren't very nice, but should be good enough to illustrate). I hope this is useful. This is my first time reporting an issue on and making a submission to an open source project, so I apologize if something doesn't turn out to be tidy enough.
My only other question is whether there's any chance of seeing this properly fixed for Bullseye in some way in the future.
--- ../original/nemo-4.8.6/libnemo-private/nemo-directory-async.c 2021-03-05 08:57:48.000000000 -0500
+++ libnemo-private/nemo-directory-async.c 2021-10-21 22:27:11.000000000 -0400
@@ -3902,7 +3902,7 @@
gboolean res;
GdkPixbuf *pixbuf, *pixbuf2;
GdkPixbufLoader *loader;
- gsize chunk_len;
+ gsize chunk_len = 10000;
pixbuf = NULL;
loader = gdk_pixbuf_loader_new ();
@@ -3912,12 +3912,20 @@
/* For some reason we have to write in chunks, or gdk-pixbuf fails */
res = TRUE;
- while (res && file_len > 0) {
+ while (res && file_len > chunk_len) {
+ res = gdk_pixbuf_loader_write (loader, (guchar *) file_contents, chunk_len, NULL);
+ file_contents += chunk_len;
+ file_len -= chunk_len;
+ }
+
+ if (res && file_len != 0)
+ {
chunk_len = file_len;
res = gdk_pixbuf_loader_write (loader, (guchar *) file_contents, chunk_len, NULL);
file_contents += chunk_len;
file_len -= chunk_len;
}
+
if (res) {
res = gdk_pixbuf_loader_close (loader, NULL);
}
//needs libgdk-pixbuf2.0-dev installed.
//Compile with g++ mre_original.cpp -L/usr/lib/x86_64-linux-gnu/ -lgdk_pixbuf-2.0 -lglib-2.0 -lgobject-2.0 -I/usr/include/gdk-pixbuf-2.0/gdk-pixbuf/ -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0/ -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -o original
#include <stdio.h>
#include <iostream>
#include "gdk-pixbuf.h"
#include <vector>
using namespace std;
static GdkPixbuf *
get_pixbuf_for_content (int file_len,
char *file_contents);
int main(int argc, char *argv[])
{
if(argc != 2)
{
printf("wrong number of arguments\n");
exit(123);
}
char *filePath=argv[1];
FILE *fp = fopen(filePath, "rb");
if(fp==nullptr)
{
printf("error openning file\n");
exit(123);
}
fseek(fp, 0, SEEK_END);
int size = ftell(fp);
fseek(fp, 0, SEEK_SET);
std::vector<char> fileContents;
fileContents.resize(size);
char * ptr = fileContents.data();
int readQty = fread(ptr, 1, size, fp);
fclose(fp);
if(readQty != size)
{
printf("error reading file: read: %d, size: %d\n", readQty, size);
exit(123);
}
get_pixbuf_for_content(size, ptr);
return 0;
}
static GdkPixbuf *
get_pixbuf_for_content (int file_len,
char *file_contents)
{
gboolean res;
GdkPixbuf *pixbuf, *pixbuf2;
GdkPixbufLoader *loader;
gsize chunk_len;
pixbuf = NULL;
loader = gdk_pixbuf_loader_new ();
/* For some reason we have to write in chunks, or gdk-pixbuf fails */
res = TRUE;
while (res && file_len > 0) {
chunk_len = file_len;
res = gdk_pixbuf_loader_write (loader, (guchar *) file_contents, chunk_len, NULL);
file_contents += chunk_len;
file_len -= chunk_len;
}
if (res) {
res = gdk_pixbuf_loader_close (loader, NULL);
}
if (res) {
pixbuf = (GdkPixbuf *) g_object_ref (gdk_pixbuf_loader_get_pixbuf (loader));
}
g_object_unref (G_OBJECT (loader));
if (pixbuf) {
pixbuf2 = gdk_pixbuf_apply_embedded_orientation (pixbuf);
g_object_unref (pixbuf);
pixbuf = pixbuf2;
}
return pixbuf;
}
//needs libgdk-pixbuf2.0-dev installed.
//Compile with g++ mre_fixed.cpp -L/usr/lib/x86_64-linux-gnu/ -lgdk_pixbuf-2.0 -lglib-2.0 -lgobject-2.0 -I/usr/include/gdk-pixbuf-2.0/gdk-pixbuf/ -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0/ -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -o fixed
#include <stdio.h>
#include <iostream>
#include "gdk-pixbuf.h"
#include <vector>
using namespace std;
static GdkPixbuf *
get_pixbuf_for_content (int file_len,
char *file_contents);
int main(int argc, char *argv[])
{
if(argc != 2)
{
printf("wrong number of arguments\n");
exit(123);
}
char *filePath=argv[1];
FILE *fp = fopen(filePath, "rb");
if(fp==nullptr)
{
printf("error openning file\n");
exit(123);
}
fseek(fp, 0, SEEK_END);
int size = ftell(fp);
fseek(fp, 0, SEEK_SET);
std::vector<char> fileContents;
fileContents.resize(size);
char * ptr = fileContents.data();
int readQty = fread(ptr, 1, size, fp);
fclose(fp);
if(readQty != size)
{
printf("error reading file: read: %d, size: %d\n", readQty, size);
exit(123);
}
get_pixbuf_for_content(size, ptr);
return 0;
}
static GdkPixbuf *
get_pixbuf_for_content (int file_len,
char *file_contents)
{
gboolean res;
GdkPixbuf *pixbuf, *pixbuf2;
GdkPixbufLoader *loader;
int chunk_len;
pixbuf = NULL;
loader = gdk_pixbuf_loader_new ();
/* For some reason we have to write in chunks, or gdk-pixbuf fails */
res = TRUE;
chunk_len = 10000;
while (res && file_len > chunk_len) {
res = gdk_pixbuf_loader_write (loader, (guchar *) file_contents, chunk_len, NULL);
file_contents += chunk_len;
file_len -= chunk_len;
}
if(file_len != 0)
{
chunk_len = file_len;
res = gdk_pixbuf_loader_write (loader, (guchar *) file_contents, chunk_len, NULL);
file_contents += chunk_len;
file_len -= chunk_len;
}
if (res) {
res = gdk_pixbuf_loader_close (loader, NULL);
}
if (res) {
pixbuf = (GdkPixbuf *) g_object_ref (gdk_pixbuf_loader_get_pixbuf (loader));
}
g_object_unref (G_OBJECT (loader));
if (pixbuf) {
pixbuf2 = gdk_pixbuf_apply_embedded_orientation (pixbuf);
g_object_unref (pixbuf);
pixbuf = pixbuf2;
}
return pixbuf;
}
Reply to: