Ensure we handle cogl functions that return NULL in data_to_cogl_texture

[gerow]

I've seen some cases with users experiencing a crash in Cinnamon with a stack trace indicating cogl_famebuffer_allocate was getting called with a null framebuffer. This doesn't fix whatever underlying issue was causing that, but handles the error case to ensure we at least don't crash.

[clefebvre]

Hi, do you have a way of reproducing the issue?

[dalcde]

Not an actual, issue, but should g_clear_error be inside the else block above it?

It wouldn't make a difference, but seems to make more sense like that

[gerow]

@clefebvre Unfortunately I haven't been able to find a way to reproduce it reliably. A few of our users seem to report it happening somewhat randomly although the most common case seems to be after unlocking the screen. Either way, here's a retrace of it happening:

#0 cogl_framebuffer_get_winsys (framebuffer=framebuffer@entry=0x0) at ./cogl-framebuffer.c:199
No locals.
#1 0x00007ff555357b1f in cogl_framebuffer_allocate (framebuffer=framebuffer@entry=0x0, error=error@entry=0x7fffdcdc3238) at ./cogl-framebuffer.c:708
onscreen = 0x0
winsys =
ctx =
#2 0x00007ff558a61902 in data_to_cogl_handle (data=0x7ff5080eceb0 "5Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\065Mq\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Lp\377\071\207\377T\217\272\377b\244\312\377m\261\323\377u\267\325\377u\257\314\377h\233\275\377Jj\215\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\064Ko\377\063Jm\377\063Jm\377\063Jm\377\063Jm\377\064Jo\377F{\252\377"..., has_alpha=1, width=22, height=20, rowstride=88, add_padding=) at st/st-texture-cache.c:595
texture = 0x7ff55b2e2fb0
offscreen = 0x0
clear_color = {private_member_red = 80 'P', private_member_green = 60 '<', private_member_blue = 114 'r', private_member_alpha = 91 '[', private_member_padding0 = 32757, private_member_padding1 = 1487280708, private_member_padding2 = 32757}
size = 22
error = 0x0
#3 0x00007ff558a61a68 in pixbuf_to_cogl_handle (pixbuf=pixbuf@entry=0x7ff55b723c50, add_padding=) at st/st-texture-cache.c:633
No locals.
#4 0x00007ff558a61b24 in finish_texture_load (data=data@entry=0x7ff5594d0b70, pixbuf=0x7ff55b723c50) at st/st-texture-cache.c:677
iter =
cache = 0x7ff55955da00
texdata = 0x0
#5 0x00007ff558a61c6e in on_icon_loaded (source=0x7ff55b372580, result=0x7ff55b5f6530, user_data=0x7ff5594d0b70) at st/st-texture-cache.c:723
pixbuf = 0x7ff55b723c50
#6 0x00007ff553a898db in g_task_return_now (task=0x7ff55b5f6530) at /build/buildd/glib2.0-2.40.2/./gio/gtask.c:1076
No locals.
#7 0x00007ff553a898f9 in complete_in_idle_cb (task=0x7ff55b5f6530) at /build/buildd/glib2.0-2.40.2/./gio/gtask.c:1085
No locals.
#8 0x00007ff557ab3ce5 in g_main_dispatch (context=0x7ff559446a00) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3064
dispatch = 0x7ff557ab0bc0 <g_idle_dispatch>
prev_source = 0x0
was_in_call = 0
user_data = 0x7ff55b5f6530
callback = 0x7ff553a898f0 <complete_in_idle_cb>
cb_funcs = 0x7ff557d71920 <g_source_callback_funcs>
cb_data = 0x7ff50858f1d0
need_destroy =
source = 0x7ff50800cb70
current = 0x7ff5594ac280
i = 71
#9 g_main_context_dispatch (context=context@entry=0x7ff559446a00) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3663
No locals.
#10 0x00007ff557ab4048 in g_main_context_iterate (context=0x7ff559446a00, block=block@entry=1, dispatch=dispatch@entry=1, self=) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3734
max_priority = 0
timeout = 0
some_ready = 1
nfds =
allocated_nfds = 8
fds = 0x7ff55c70c400
#11 0x00007ff557ab430a in g_main_loop_run (loop=0x7ff559446d30) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3928
FUNCTION = "g_main_loop_run"
#12 0x00007ff558571f17 in meta_run () from /usr/lib/x86_64-linux-gnu/libmuffin.so.0
No symbol table info available.
#13 0x00007ff558ece94f in main (argc=1, argv=0x7fffdcdc35e8) at main.c:288
ctx =
error = 0x0
ecode =

@dalcde Makes sense to me. I've amended the commit to move it inside the else block.

[gerow]

I should note that the stack trace is from 2.2.16.

[mtwebster]

Looks ok to me, tested, no obvious issue. I'm only curious how this never manifested sooner than it did.