[Aptitude-devel] Bug#810020: aptitude: sometimes crashes when reading changelogs

[Manuel A. Fernandez Montecelo]

Control: tags -1 + pending


Hi,

2016-01-12 23:39 Axel Beckert:
>Control: tag -1 + confirmed
>
>Hi Andreas,
>
>Andreas Cadhalpun wrote:
>> On 05.01.2016 18:46, Andreas Cadhalpun wrote:
>> > since upgrading to apt > 1.1 aptitude sometimes crashes, when reading changelogs.
>> > So far it happened four times in the last month.
>>
>> I have found a quite reliable way to reproduce the crash:
>>  * open the aptitude ncurses interface
>>  * select one package (e.g. aptitude)
>>  * press 'C' multiple times (at least twice) in a row, fast
>>  * watch the segfault...
>
>Indeed. With those instructions I can reproduce the issue with
>aptitude 0.7.5-3 on amd64.
>
>Thanks for bug report and the followup with more details.

This turned out to be incredibly complicated to debug, and I couldn't
find out in the end the exact cause of the crash after several
afternoons and evenings looking into it.  Threading make this kind of
debugging very complicate.

I believe that the crashes don't happen anymore with this fix, though.
I was tempted to rewrite the whole thing, with goodies like the ability
to use pkgAcqChangelog with lowered privileges (user "_apt"), but I
don't want to delay a new release for weeks or months -- this doesn't
seem like a minor task at all.

A bit of more detail follows, in what I posted in the commit.


  * Do not download changelogs when they are already being downloaded
    (Closes: #810020)

    Until now, pending changelogs kept being queued (e.g. pressing 'C'
    repeatedly in the curses interface spawned many downloads), and in the
    latest versions of apt (>= 1.1) it caused aptitude to crash.  In previous
    versions of aptitude/apt I did not crash, but it caused weird errors
    ("download queue being destroyed").  Maybe internal changes within apt cause
    now crashes when they were errors before (not necessarily apt's fault, maybe
    it's a misuse from aptitude).

    The errors already seen in previous versions seem to be caused by aptitude
    code, for example because all downloads with the same URI are deleted from
    some internal structures of aptitude.  With this "fix" at least repeated
    downloads are not repeated (*), which is a nice feature in general, and
    seems to solve this problem.

      (*) Still, sometimes the changelog is downloaded and shown twice, maybe
      this is because of sync problems between background threads, or maybe
      because sometimes the download happens so fast that the first is already
      downloaded by the time that the second is queued (this fix only prevents
      to queue when "to-be-started" or "active-downloads", not when it was
      downloaded previously).

    More in general, the code related with downloading files/changelogs is
    incredibly complex and entangled, spans many files and dozens of classes
    nested again and using inheritences and overlapping code, and it uses a
    home-grown implementation of threads in cwidget copied or based on POSIX
    threads.  After several multi-hour sessions trying to analyse the point of
    the crash and improve this situation, and failing to wrap my head around it,
    I think that it's probably better to implement a new way to download
    changelogs, which I believe that can be simplified significantly, and
    hopefully in this case other problems will be easier to fix, and we can
    start to use pkgAcqChangelog and avoid the warnings about not being able to
    change the user to "_apt".  But this has to wait for another release.


-- 
Manuel A. Fernandez Montecelo <manuel.montezelo at gmail.com>