segfault when clicking on Source

[sssoleileraaa]

Fixes #248 where the client crashes when a user clicks on a source from the sources list.

When debugging the client I could see that crash happened when adding the widget to the view_layout:

(Pdb) w
  /usr/lib/python3.5/runpy.py(193)_run_module_as_main()
-> "__main__", mod_spec)
  /usr/lib/python3.5/runpy.py(85)_run_code()
-> exec(code, run_globals)
  /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/__main__.py(3)<module>()
-> run()
  /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/app.py(187)run()
-> start_app(args, qt_args)
  /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/app.py(180)start_app()
-> sys.exit(app.exec_())
  /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/gui/main.py(167)on_source_changed()
-> self.show_conversation_for(self.current_source)
  /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/gui/main.py(182)show_conversation_for()
-> self.main_view.set_conversation(conversation_container)
> /home/creviera/workspace/freedomofpress/securedrop-client/securedrop_client/gui/widgets.py(177)set_conversation()
-> self.view_layout.addWidget(widget)

TODO: Still more research needs to be done as to why Qt crashes when a widget isn't set to visible.

[redshiftzero]

OK so in reviewing this I noticed that I could not reproduce the original bug on the version of pyqt5 specified on latest master, but if I first:

pipenv install pyqt5==5.12

then I can reproduce and I confirm this PR does fix the segfault 🎉.

It's possible this is a qt bug, but it's also possible that we are using pyqt improperly i.e. we are referencing the Python wrapper for an object that qt has deleted, check out this document for more details on the interplay between pyqt and qt, that might be a useful starting point.

[sssoleileraaa]

@redshiftzero Thanks for linking me to that doc! This sorta seems like an ownership issue since the segfault is happening when addWidget is called, which transfers ownership of widget, but our segfault only seems to happen when we try to add hidden widgets, e.g.:

(Pdb) button = QPushButton("push")
(Pdb) button.isVisible()
False
(Pdb) self.view_layout.addWidget(widget)
[1]    24061 segmentation fault  ./run.sh

When stepping though the code, I noticed that setting widget (our SourceConversationWrapper object) to visible caused a new window to pop up, as you can see in the screenshot below. ANd it goes away once it's added to view_layout. If it weren't for this bug, I would think that it would be better to wait until after it's added to view_layout before setting the widget to visible. Even though the code execution is fast enough that you won't really see this window, I think it's worth trying to come up with another solution to this segfault issue.

Another solution might require more of a redesign, but hopefully there's something in between rewriting SourceConversationWrapper and MainView and just setting a QWidget variable.

[sssoleileraaa]

More info on troubleshooting this issue:

• I'm able to repro every single time by clicking on a source immediately after logging in.
• I noticed if I waited for about 5 seconds the segfault doesn't happen
• It looks like the segfault happens when we try to add a widget to the view_layout before a data sync where we fetch and decrypt new submissions

@heartsucker do you have any thoughts or ideas on what could be happening?

[heartsucker]

@creviera I think the best I can guess is that we're having issues with the parent/child relationships being correctly established. From the docs, it looks like this should raise a RuntimeError saying something like Underlying C/C++ object has been freed or something like that. Since this is a segfault, it seems like the error is more serious. That said, the issue may be that the bindings don't perfectly catch everything. I am not particularly familiar with Python-to-C bindings nor with the underlying C++ Qt code. This is just making some wild ass semi-educated guesses. I think generally we should ensure that all QWidgets have correctly established their parent/child relationships, which I think we don't currently do in all cases.

[redshiftzero]

So upon further investigation, this appears to be an upstream issue. Check out this branch and see if you can trigger the segmentation fault. The investigation process here inspecting the core dump for the segfault:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
[...entirely obtuse part snipped out...]
2   QtWidgets                     	0x000000010a6bb4ef QWidgetPrivate::setStyle_helper(QStyle*, bool) + 639
3   QtWidgets                     	0x000000010a6bb239 QWidgetPrivate::inheritStyle() + 457
4   QtWidgets                     	0x000000010a6b5e39 QWidget::setParent(QWidget*, QFlags<Qt::WindowType>) + 1705
5   QtWidgets                     	0x000000010a6a8d43 QLayout::addChildWidget(QWidget*) + 211
6   QtWidgets                     	0x000000010a6a02ce QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) + 62
7   QtWidgets.so                  	0x000000010a3475a5 meth_QBoxLayout_addWidget(_object*, _object*, _object*) + 197

Searching the QTbug tracker for issues involving setStyle_helper, there's an ongoing upstream bug (QTBUG-69204) in the handling of QStyleSheetStyle's updateObjects() causing segfaults. If this is indeed the problem we are hitting, then we should be able to disable all our CSS styling and everything should work, which is what is done in the above branch, demonstrating that we're hitting an upstream bug with CSS styling.

Now there is also the possibility there is another upstream issue with the CSS stylesheets, the way to fully validate that QTBUG-69204 is the culprit would be to apply the patch here, recompile qt and then verify that master does not hit the segfault.

Then going forward, we can do one of the following:

1. Disable CSS styles for now (bad, we want to do more UI work).
2. Have developers apply the patch and compile qt from source (bad dev agility).
3. Determine if the fix in this PR is indeed a workaround, commit it with a note (and followup issue) that we should revert when qt 5.12.1 comes out with the fix - this is in my opinion the best option such that we can proceed with additional CSS styling work without introducing headaches for developers/testers.

[sssoleileraaa]

@redshiftzero, ready for final review

[redshiftzero]

confirming on this branch that I don't hit the segfault on PyQt 5.12, this should unblock us until PyQt 5.12.1 is released