[PyQt] Use after free bug in pyqt 5.8.0 / sip 4.19.1

[dequis]

On 30 July 2017 at 21:54, dequis <dx at dxzone.com.ar> wrote:

> [resending this without the attachment, sorry if it ends up being posted
> twice]
>
> On 9 March 2017 at 17:46, dequis <dx at dxzone.com.ar> wrote:
>
>> Hi.
>>
>> I got a crash with anki (git version with pyqt5 instead of 4). I'm not
>> sure how to reproduce it, but [snip]
>>
>
> Hey there, it's me again with the anki crashes. Still happening with pyqt
> 5.9. I took a break from using the anki desktop app for a few months but
> now it's time to deal with it again.
>
> I still don't know how to intentionally reproduce it other than "just use
> the app normally for a while", but using the app normally for a while works
> (Which I'd totally recommend since anki is an excellent app, but that's not
> a very reliable way to reproduce it). Takes hours to reproduce but looks
> like it happens roughly once every 12-48 hours.
>
> So I prepared a bit better this time:
>
> - got debug symbols for everything (thanks the-compiler for the repo!)
> - patched the sip build scripts to not strip on 'make install'
> - installed the excellent python gdb extensions
> - replaced malloc with a tcmalloc_debug to make it crash more reliably
> - set PYTHONMALLOC=malloc
> - and ran the whole thing under rr
>
> Way better than valgrind, since I have time travel now, and I can replay
> this as many times as I want. I gave it a shot to try to extract as much
> info as I could.
>
> Here's the annotated gdb/rr session:
>
> http://dump.dequis.org/G21sm.txt
>
> And here's what I learnt:
>
> - The object being freed is EditCurrent, a subclass of QDialog (I think
> it's the dialog opened from the edit button during a review)
> - The free happens during garbage collection because it needs to break a
> reference cycle between EditCurrent and Editor
> - Some interesting interactions with the code that calls javascript to do
> "saveNow"
>
> It got hairy at some point and I didn't reach the initial allocation of
> the object - lots of incref/decref in code related to saveNow. I'll
> continue later.
>
> Also worth noting that i'm using a slightly old git revision of anki,
> 43a662a installed april 15. Didn't want to upgrade just in case the bug
> stopped happening.
>
> One recent anki commit caught my attention, "fix duplicate constructor
> call in editcurrent", three days ago, removes a duplicate call to "
> QDialog.__init__". Who knows if it's relevant. It takes forever to find
> out so I'd rather stay with what I have.
>
> Any suggestions on how to continue debugging this would be appreciated. I
> got some new ideas on how to reproduce it, but nothing seems to work so far.
>

Okay, so, the good news is that I got decent repro steps now.

The bad (but also good) news is that this is anki's fault and it was indeed
fixed by removing that duplicate constructor call.

https://github.com/dae/anki/commit/5ef1692c781356c23d7f3c1fecbb61cb57f684cb

I'm not sure where you normally set the boundary of where to blame the
application, but I hope this is some kind of misbehavior pyqt can detect
and fail early instead of messing up reference counts.

Requirements:

- Python 3.6
- Anki 2.1.0beta4 or current git with the commit above reverted.
- Have at least one deck with one card ("add" in the main window)
- Optional: gperftools or an equivalent package containing tcmalloc (other
mallocs are less reliable for debugging)

Procedure:

1. Start anki with:

$ LD_PRELOAD=/usr/lib/libtcmalloc_debug.so PYTHONMALLOC=malloc python3
/usr/bin/anki

Or using glibc's malloc, which is less reliable:

$ MALLOC_PERTURB_=255 PYTHONMALLOC=malloc python3 /usr/bin/anki

2. Click the deck name
3. Click study now
4. Click edit (bottom left)
5. Close the edit window
6. Press ctrl+: (colon)
7. Enter "gc.collect()"
8. Press ctrl+enter
9. Click edit again
10. If using MALLOC_PERTURB_ and it doesn't crash, repeat from step 5 a
couple of times.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.riverbankcomputing.com/pipermail/pyqt/attachments/20170801/42ab942f/attachment.html>