This is the mail archive of the cygwin-apps mailing list for the Cygwin project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [setup topic/libsolv] Crash after incomplete download

On 02/11/2017 19:25, Ken Brown wrote:
On 11/2/2017 1:22 PM, Jon Turney wrote:
On 01/11/2017 20:38, Ken Brown wrote:
If there is a download failure and the user clicks Yes in response to "Download Incomplete.  Try again?", then setup will crash.  The crash occurs at because i->source() is NULL.

Thanks for finding all these problems!

I haven't yet analyzed this in further detail, but the crux of the issue seems to be that we call do_ini_thread a second time without having cleaned out the package database and the libsolv pool.

This is probably a symptom of a more general problem, which is that we haven't thought carefully (or at least I haven't) about what happens when we visit certain pages for a second time, after the libsolv pool has been created.

The fact that this isn't considered at all at the moment makes me wonder if it's working correctly in this situation currently.  But, yeah, I agree that packagedb state should be cleared in this case.

I tried with the attached, but it still segfaults at the same place.

This seems to be because all the packagedb prep (including fixup_source_package_ids) is in OnInit() (which is only called once per page, but lazily), rather than OnActivate(), so probably some more restructuring is needed there :(

I guess we could move all that prep to OnActivate() but make sure it's only run when we've gotten to the chooser page from the site page (and not because the user pressed Back on the prereq page).  What about adding a 'prepped' data member to packagedb and using this to decide whether to run the prep code?

An attempt at this to follow, but this ended up touching rather more stuff than I wanted to, so I'm sure something else has broken :(

+packagedb::init ()
+  installeddbread = 0;
+  installeddbver = 0;
+  packages.clear();
+  sourcePackages.clear();
+  categories.clear();
+  solver.clear();
+  basepkg = packageversion();
+  dependencyOrderedPackages.clear();

I think you also need solution.clear(), where the latter does essentially what SolverSolution::~SolverSolution() does.  (Or is it enough to just set solv = NULL?)  Otherwise, solv will be an invalid pointer when update is next called.

Yes, you're right, of course.

The initialization all gets a bit obscure, maybe making solver and solversolution directly members of this singleton isn't the right approach...

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]