From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21500 invoked by alias); 2 Nov 2017 19:25:21 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 21486 invoked by uid 89); 2 Nov 2017 19:25:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=pressed, unattended, visit, site X-HELO: limerock03.mail.cornell.edu Received: from limerock03.mail.cornell.edu (HELO limerock03.mail.cornell.edu) (128.84.13.243) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 02 Nov 2017 19:25:19 +0000 X-CornellRouted: This message has been Routed already. Received: from authusersmtp.mail.cornell.edu (granite4.serverfarm.cornell.edu [10.16.197.9]) by limerock03.mail.cornell.edu (8.14.4/8.14.4_cu) with ESMTP id vA2JPHwU006533 for ; Thu, 2 Nov 2017 15:25:17 -0400 Received: from [10.13.22.3] (65-112-130-194.dia.static.qwest.net [65.112.130.194]) (authenticated bits=0) by authusersmtp.mail.cornell.edu (8.14.4/8.12.10) with ESMTP id vA2JPFEg024402 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT) for ; Thu, 2 Nov 2017 15:25:16 -0400 Subject: Re: [setup topic/libsolv] Crash after incomplete download To: cygwin-apps@cygwin.com References: <4966bc07-8a00-3324-6b6f-4da2213e974c@cornell.edu> <2ad6ea16-ef19-ae4e-757f-9e9b882aba75@dronecode.org.uk> From: Ken Brown Message-ID: <2b7291f9-3df7-7f3b-6326-79bc939f595f@cornell.edu> Date: Thu, 02 Nov 2017 19:25:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <2ad6ea16-ef19-ae4e-757f-9e9b882aba75@dronecode.org.uk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-PMX-Cornell-Gauge: Gauge=X X-PMX-CORNELL-AUTH-RESULTS: dkim-out=none; X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00005.txt.bz2 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 PickView.cc:447 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? >> Before I work further on this, I have a UI question.  Is it really >> reasonable that we go back to the mirror selection page after >> "Download incomplete"?  I understand the rationale, which is that the >> user might want to try a different mirror after a download failure. >> But I personally have always found this annoying.  I would rather just >> retry the download, which is what the message ("Download Incomplete. >> Try again?") suggests is going to happen. > > This makes perfect sense to me. > > The path that an unattended install takes out of there is particularly > convoluted, as well. > > I suspect there's other terribleness in this area, like if you choose > "No", the incomplete package download is left in the local package cache > to fail it's checksum check on the next run... > > +void > +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. Ken