On Sun, Nov 25, 2007 at 03:34:35PM +0200, Sami Liedes <[EMAIL PROTECTED]> was
heard to say:
> On Tue, Nov 20, 2007 at 03:09:27PM +0200, Sami Liedes wrote:
> > On Mon, Nov 19, 2007 at 07:26:42AM -0800, Daniel Burrows wrote:
> > > I don't know for sure that they're the same. Breaking in ::Lookup()
> > > just means it got an invalid iterator. I've found a possible bug that
> > > could (perhaps) explain your crash and the crashes that happen on update
> > > or after an upgrade, but it won't explain the crashes that happen when
> > > the user presses 'f', at least as far as I can tell. Could you let me
> > > know if the attached patch helps?
> >
> > Nope. I upgraded to 0.4.9-1 with your patch by building cwidget and
> > aptitude (the newest aptitude for amd64 in sid is 0.4.7-1). I still
> > get the same crash.
>
> After having used 0.4.9-1 with the patch for a while, I can say it
> seems to crash much more seldom now.
So, I just got some more time to look at this, and it looks like I was
stupid when I wrote the last patch and forgot to initialize the flag
controlling whether we try to read the lastPkg and lastVer fields.
Could you let me know if this patch helps at all? (it combines my
previous patch with the one-line fix to actually initialize have_pkg to
false)
Thanks,
Daniel
diff -r 346408273a37 -r 923ccf67a85f src/pkg_view.cc
--- a/src/pkg_view.cc Sun Nov 18 06:41:05 2007 +0000
+++ b/src/pkg_view.cc Tue Nov 20 07:30:01 2007 -0800
@@ -301,6 +301,12 @@ class info_area_multiplex:public cw::mul
cw::text_layout_ref reasons;
cw::table_ref reasons_table;
+ /** True if the package members are valid.
+ *
+ * This needs to be here because it's not always safe to check end()
+ * if the cache has been reloaded.
+ */
+ bool have_pkg;
pkgCache::PkgIterator lastPkg;
pkgCache::VerIterator lastVer;
wstring lastDesc;
@@ -311,6 +317,11 @@ class info_area_multiplex:public cw::mul
* to the widget we switched away from; otherwise, it is \b NULL.
*/
cw::widget_ref autoswitch;
+
+ void clear_package()
+ {
+ have_pkg = false;
+ }
protected:
info_area_multiplex(const hier_editor_ref &_editor,
@@ -327,6 +338,8 @@ protected:
reasons(_reasons), reasons_table(_reasons_table),
hadBreakage(false), autoswitch(NULL)
{
+ cache_closed.connect(sigc::mem_fun(*this,
+ &info_area_multiplex::clear_package));
package_states_changed.connect(sigc::mem_fun(*this,
&info_area_multiplex::reset_package));
}
@@ -392,6 +405,7 @@ public:
// moved to a broken package.
if(hasBreakage &&
(!hadBreakage ||
+ !have_pkg ||
!(pkg==lastPkg && ver==lastVer)) &&
aptcfg->FindB(PACKAGE "::UI::Auto-Show-Reasons", true))
{
@@ -404,7 +418,7 @@ public:
// We always set the package anyway in case something changed,
// but only scroll to the top in this case:
- if(pkg!=lastPkg || ver!=lastVer)
+ if(!have_pkg || pkg != lastPkg || ver != lastVer)
{
lastPkg=pkg;
lastVer=ver;
@@ -420,6 +434,7 @@ public:
}
hadBreakage=hasBreakage;
+ have_pkg = true;
}
/** Cycles the multiplex, taking autoswitch behavior into account. */
@@ -444,7 +459,8 @@ public:
*/
void reset_package()
{
- set_package(lastPkg, lastVer);
+ if(have_pkg)
+ set_package(lastPkg, lastVer);
}
/** Set the description directly, without reference to a package.
@@ -462,9 +478,9 @@ public:
description->set_fragment(make_desc_fragment(s));
reasons->set_fragment(cw::sequence_fragment(make_desc_fragment(s),
- cw::newline_fragment(),
- nopackage(),
- NULL));
+ cw::newline_fragment(),
+ nopackage(),
+ NULL));
description->move_to_top();
reasons->move_to_top();
diff -r bd548a8d1d45 src/pkg_view.cc
--- a/src/pkg_view.cc Fri Nov 23 08:53:02 2007 -0800
+++ b/src/pkg_view.cc Thu Nov 29 17:58:18 2007 -0800
@@ -336,7 +336,7 @@ protected:
description(_description), description_table(_description_table),
why(_why), why_table(_why_table),
reasons(_reasons), reasons_table(_reasons_table),
- hadBreakage(false), autoswitch(NULL)
+ have_pkg(false), hadBreakage(false), autoswitch(NULL)
{
cache_closed.connect(sigc::mem_fun(*this,
&info_area_multiplex::clear_package));
