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 780707 - Crashes going back and forth between epubs
Crashes going back and forth between epubs
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: books
3.24.x
Other Linux
: Normal normal
: ---
Assigned To: GNOME Books Maintainers
GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-03-29 21:24 UTC by Bastien Nocera
Modified: 2017-03-31 05:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libgepub: Add guards to public functions (7.95 KB, patch)
2017-03-30 10:55 UTC, Bastien Nocera
committed Details | Review
pdf-loader: make sure to cancel load job (2.69 KB, patch)
2017-03-31 02:15 UTC, Cosimo Cecchi
committed Details | Review
documents: factor out a common function (1.92 KB, patch)
2017-03-31 02:15 UTC, Cosimo Cecchi
committed Details | Review

Description Bastien Nocera 2017-03-29 21:24:48 UTC
libgepub-0.4-2.fc26.x86_64
gnome-books-3.23.91-1.fc26.x86_64

  • #0 gepub_doc_get_current
    at gepub-doc.c line 465
  • #1 gepub_doc_get_current_with_epub_uris
    at gepub-doc.c line 479
  • #2 reload_current_chapter
    at gepub-widget.c line 191
  • #3 gepub_widget_set_doc
    at gepub-widget.c line 224
  • #0 gepub_doc_get_current
    at gepub-doc.c line 465
$2 = (GepubDoc *) 0x55f92335f540
(gdb) p doc->page
$3 = (GList *) 0x0

Oops
Comment 1 Cosimo Cecchi 2017-03-30 03:12:39 UTC
How do you reproduce this? I tried with a few epubs and moving back/forward between their pages and could not make it crash.
Comment 2 Bastien Nocera 2017-03-30 10:33:29 UTC
(In reply to Cosimo Cecchi from comment #1)
> How do you reproduce this? I tried with a few epubs and moving back/forward
> between their pages and could not make it crash.

Not between pages, but between epubs. I can't reproduce this on my desktop machine, possibly because it's too quick.
Comment 3 Bastien Nocera 2017-03-30 10:55:52 UTC
Created attachment 348984 [details] [review]
libgepub: Add guards to public functions

So as to catch incorrect usage of the APIs as soon as possible.
Comment 4 Bastien Nocera 2017-03-30 10:56:23 UTC
Patch for pre-rust libgepub, need to see whether that catches something in my reproducer.
Comment 5 Bastien Nocera 2017-03-30 12:25:01 UTC
Reproducer:
- open comics (evince view)
- go back before it's finished loading
- click on epub view

With the above patch:
** (org.gnome.Books:6333): CRITICAL **: gepub_doc_get_current: assertion 'doc->page != NULL' failed

(gdb) call gjs_dumpstack()
== Stack trace for context 0x555555782050 ==
EPUBView<.onLoadFinished@resource:///org/gnome/Books/js/epubview.js:89:9
wrapper@resource:///org/gnome/gjs/modules/lang.js:178:22
_emit@resource:///org/gnome/gjs/modules/signals.js:126:27
DocumentManager<._onDocumentLoaded@resource:///org/gnome/Books/js/documents.js:1371:9
wrapper@resource:///org/gnome/gjs/modules/lang.js:178:22
DocCommon<.loadLocal/<@resource:///org/gnome/Books/js/documents.js:626:21
main@resource:///org/gnome/Books/js/main.js:47:12
run@resource:///org/gnome/gjs/modules/package.js:192:12
@/usr/bin/gnome-books:6:1

$1 = 1433768880
(gdb) bt
  • #0 _g_log_abort
    from /lib64/libglib-2.0.so.0
  • #1 g_logv
    from /lib64/libglib-2.0.so.0
  • #2 g_log
    from /lib64/libglib-2.0.so.0
  • #3 gepub_doc_get_current
    at gepub-doc.c line 490
  • #4 gepub_doc_get_current_with_epub_uris
    at gepub-doc.c line 511
  • #5 reload_current_chapter
    at gepub-widget.c line 193
  • #6 gepub_widget_set_doc
    at gepub-widget.c line 228
  • #7 g_object_set_property
    from /lib64/libgobject-2.0.so.0
  • #8 ??
    from /lib64/libgjs.so.0
  • #9 js::Shape::set(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JSObject*>, bool, JS::MutableHandle<JS::Value>)
    from /lib64/libmozjs-38.so
  • #10 NativeSet(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JSObject*>, JS::Handle<js::Shape*>, bool, JS::MutableHandle<JS::Value>)
    from /lib64/libmozjs-38.so
  • #11 js::NativeSetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, js::QualifiedBool, JS::MutableHandle<JS::Value>, bool)
    from /lib64/libmozjs-38.so
  • #12 SetObjectProperty(JSContext*, JSOp, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
    from /lib64/libmozjs-38.so
  • #13 Interpret(JSContext*, js::RunState&)
    from /lib64/libmozjs-38.so
  • #14 js::RunScript(JSContext*, js::RunState&)
    from /lib64/libmozjs-38.so
  • #15 js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)
    from /lib64/libmozjs-38.so
  • #16 js_fun_apply(JSContext*, unsigned int, JS::Value*)
    from /lib64/libmozjs-38.so
  • #17 js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)
    from /lib64/libmozjs-38.so
  • #18 js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>)
    from /lib64/libmozjs-38.so
  • #19 js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)
    from /lib64/libmozjs-38.so
  • #20 ??
  • #21 ??

Comment 6 Bastien Nocera 2017-03-30 12:33:40 UTC
I think you can make this much more easily by making gd_pdf_loader_load_uri_async() take ages. Or loading a huge CBZ/CBR file.
Comment 7 Cosimo Cecchi 2017-03-31 02:14:58 UTC
Managed to reproduce and pushed a fix to master.

The following fixes have been pushed:
e462682 pdf-loader: make sure to cancel load job
1b1346f documents: factor out a common function
Comment 8 Cosimo Cecchi 2017-03-31 02:15:11 UTC
Created attachment 349024 [details] [review]
pdf-loader: make sure to cancel load job

Otherwise we will get a load-succeeded signal in the views, instead of a
cancelled error, which will end up loading a wrong document, leading to
a crash.
Comment 9 Cosimo Cecchi 2017-03-31 02:15:14 UTC
Created attachment 349025 [details] [review]
documents: factor out a common function
Comment 10 Bastien Nocera 2017-03-31 05:50:33 UTC
Comment on attachment 348984 [details] [review]
libgepub: Add guards to public functions

Pushed to libgepub master as well.

Attachment 348984 [details] pushed as 0da0ba3 - libgepub: Add guards to public functions