akrennmair/newsbeuter — pull request #127 by ittner (2014-04-14 02:11:20+00:00)
Title: Fix crash when a GUID is lost while browsing command is running

Fix an abnormal termination which happens when a GUID is lost due to the item being updated or deleted by the fetch thread when the web browser command is running in the main thread. When the browser finishes, newsbeuter attempts to find the item being read and calls abort() on failure.

This crash is more frequent when using text-mode browsers (e.g. w3m) which, unlike the graphical ones, do not return immediately.

This fix is more a hack than a proper one. The problem was exposed when a lock was added to method rss_feed::get_item_by_guid_unlocked() in commit d71336f48be10df737b811edc7796d6a9a88498e. After that, failure to find the item resulted in abort() being called. This fix brings back the dummy item introduced in commit 8dd8a153f8fd5b2fb86eb74e842ffe621c0ea4ba, avoiding the crash but, possibly, showing an empty article.

#### Follows a trace of a typical instance of this bug

ittner@elendil:~/projetos/newsbeuter-contrib$ gdb ./newsbeuter
GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
...
Reading symbols from /home/ittner/projetos/newsbeuter-contrib/newsbeuter...done.
(gdb) set environment LD_PRELOAD=/opt/libjson-c/lib/libjson-c.so.2
(gdb) run
Starting program: /home/ittn

Program received signal SIGABRT, Aborted.
0x00007ffff5a24425 in __GI_raise (sig=) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007ffff5a24425 in __GI_raise (sig=) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x00007ffff5a27b8b in __GI_abort () at abort.c:91
#2 0x000000000045b2ef in newsbeuter::rss_feed::get_item_by_guid_unlocked (this=0x7fffe071b2d0, guid=...)
at src/rss.cpp:229
#3 0x000000000045b0ff in newsbeuter::rss_feed::get_item_by_guid (this=0x7fffe071b2d0, guid=...) at src/rss.cpp:215
#4 0x0000000000491a61 in newsbeuter::itemview_formaction::process_operation (this=0x1300730,
op=newsbeuter::OP_QUIT, automatic=false, args=0x0) at src/itemview_formaction.cpp:154
#5 0x000000000047385b in newsbeuter::formaction::process_op (this=0x1300730, op=newsbeuter::OP_QUIT,
automatic=false, args=0x0) at src/formaction.cpp:139
#6 0x000000000042c05b in newsbeuter::view::run (this=0x7fffffffdb30) at src/view.cpp:239
#7 0x0000000000440741 in newsbeuter::controller::run (this=0x7fffffffdc60, argc=1, argv=0x7fffffffe078)
at src/controller.cpp:608
#8 0x00000000004086c8 in main (argc=1, argv=0x7fffffffe078) at newsbeuter.cpp:30
(gdb) list
59 in ../nptl/sysdeps/unix/sysv/linux/raise.c
(gdb) up
#1 0x00007ffff5a27b8b in __GI_abort () at abort.c:91
91 abort.c: No such file or directory.
(gdb) list
86 in abort.c
(gdb) up
#2 0x000000000045b2ef in newsbeuter::rss_feed::get_item_by_guid_unlocked (this=0x7fffe071b2d0, guid=...)
at src/rss.cpp:229
229 abort();
(gdb) list
224 return it->second;
225 }
226 LOG(LOG_DEBUG, "rss_feed::get_item_by_guid_unlocked: hit dummy item!");
227 LOG(LOG_DEBUG, "rss_feed::get_item_by_guid_unlocked: items_guid_map.size = %d", items_guid_map.size());
228 }
229 abort();
230 }
231
232 bool rss_item::has_attribute(const std::string& attribname) {
233 // LOG(LOG_DEBUG, "rss_item::has_attribute(%s) called", attribname.c_str());
(gdb) up
#3 0x000000000045b0ff in newsbeuter::rss_feed::get_item_by_guid (this=0x7fffe071b2d0, guid=...) at src/rss.cpp:215
215 return get_item_by_guid_unlocked(guid);
(gdb) list
210 return false;
211 }
212
213 std::tr1::shared_ptr rss_feed::get_item_by_guid(const std::string& guid) {
214 scope_mutex lock(&item_mutex);
215 return get_item_by_guid_unlocked(guid);
216 }
217
218 std::tr1::shared_ptr rss_feed::get_item_by_guid_unlocked(const std::string& guid) {
219 LOG(LOG_DEBUG, "get_item_by_guid_unlocked: guid = %s", guid.c_str());
(gdb) up
#4 0x0000000000491a61 in newsbeuter::itemview_formaction::process_operation (this=0x1300730,
op=newsbeuter::OP_QUIT, automatic=false, args=0x0) at src/itemview_formaction.cpp:154
154 std::tr1::shared_ptr item = feed->get_item_by_guid(guid);
(gdb) list
149 }
150
151 }
152
153 void itemview_formaction::process_operation(operation op, bool automatic, std::vector * args) {
154 std::tr1::shared_ptr item = feed->get_item_by_guid(guid);
155 bool hardquit = false;
156
157 /*
158 * whenever we process an operation, we mark the item
(gdb) up
#5 0x000000000047385b in newsbeuter::formaction::process_op (this=0x1300730, op=newsbeuter::OP_QUIT,
automatic=false, args=0x0) at src/formaction.cpp:139
139 this->process_operation(op, automatic, args);
(gdb) list
134 break;
135 case OP_PREVDIALOG:
136 v->goto_prev_dialog();
137 break;
138 default:
139 this->process_operation(op, automatic, args);
140 }
141 }
142
143 std::vector formaction::get_suggestions(const std::string& fragment) {
(gdb) up
#6 0x000000000042c05b in newsbeuter::view::run (this=0x7fffffffdb30) at src/view.cpp:239
239 fa->process_op(op);
(gdb) list
234 have_macroprefix = true;
235 set_status("macro-");
236 }
237
238 // now we handle the operation to the formaction.
239 fa->process_op(op);
240 }
241 }
242 }
243
(gdb) up
#7 0x0000000000440741 in newsbeuter::controller::run (this=0x7fffffffdc60, argc=1, argv=0x7fffffffe078)
at src/controller.cpp:608
608 v->run();
(gdb) list
603 }
604
605 formaction::load_histories(searchfile, cmdlinefile);
606
607 // run the view
608 v->run();
609
610 unsigned int history_limit = cfg.get_configvalue_as_int("history-limit");
611 LOG(LOG_DEBUG, "controller::run: history-limit = %u", history_limit);
612 formaction::save_histories(searchfile, cmdlinefile, history_limit);
(gdb) up
#8 0x00000000004086c8 in main (argc=1, argv=0x7fffffffe078) at newsbeuter.cpp:30
30 c.run(argc,argv);
(gdb) list
25
26 controller c;
27 newsbeuter::view v(&c);
28 c.set_view(&v);
29
30 c.run(argc,argv);
31
32 rsspp::parser::global_cleanup();
33
34 return 0;
(gdb) up
Initial frame selected; you cannot go up.