rework setUrl wait on eventloop

[itziakos]

This PR reworks the code to make sure that the events are disconnected.

Details:

• Do not add a runtime instance attribute _loaded
• Make sure that we disconnect events on function exit.

[codecov-io]

Codecov Report

Merging #92 into master will increase coverage by 0.06%.
The diff coverage is 90%.

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   77.22%   77.29%   +0.06%     
==========================================
  Files          18       18              
  Lines        1032     1035       +3     
  Branches      107      107              
==========================================
+ Hits          797      800       +3     
- Misses        202      203       +1     
+ Partials       33       32       -1

Impacted Files	Coverage Δ
jigna/core/proxy_qwebview.py	89.47% <90%> (+0.58%)	⬆️
jigna/utils/gui.py	95.45% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f45ad9e...6039e37. Read the comment docs.

[prabhuramachandran]

@itziakos -- just for my clarity can you please provide a bit more information as to why you are making this change? I can see that onload should take an additional boolean argument but what is wrong with the runtime instance attribute and what problem does this change solve? Thanks.

[itziakos]

Sorry, I created the PR in a haste.

On an internal application we have been experiencing segfaults on windows. I tracked down the segault and it was taking place inside setUrl

Fault handler output

Current thread 0x00001f90 (most recent call first):
  File "build\bdist.win-amd64\egg\jigna\core\proxy_qwebview.py", line 131 in setUrl
  File "build\bdist.win-amd64\egg\jigna\qt_server.py", line 82 in __init__
  File "build\bdist.win-amd64\egg\jigna\html_widget.py", line 71 in _create_server
  File "build\bdist.win-amd64\egg\jigna\html_widget.py", line 38 in __init__

It is not totally clear why it would segfault at this place but the stack trace was pointing to happening in response to the load Finished event

stack trace

QtWebKit4.dll!JSC::MarkStack::append<class JSC::JSFunction>(class JSC::WriteBarrierBase<class JSC::JSFunction> *)	C++
 	QtWebKit4.dll!JSC::CodeBlock::visitAggregate(class JSC::MarkStack &)	C++
 	QtWebKit4.dll!JSC::FunctionExecutable::visitChildren(class JSC::MarkStack &)	C++
 	QtWebKit4.dll!JSC::MarkStack::visitChildren(class JSC::JSCell *)	C++
 	QtWebKit4.dll!JSC::MarkStack::drain(void)	C++
 	QtWebKit4.dll!JSC::Heap::markRoots(void)	C++
 	QtWebKit4.dll!JSC::Heap::allocateSlowCase(unsigned __int64)	C++
 	QtWebKit4.dll!JSC::StructureChain::create(class JSC::JSGlobalData &,class JSC::Structure *)	C++
 	QtWebKit4.dll!JSC::Structure::prototypeChain(class JSC::ExecState *)	C++
 	QtWebKit4.dll!JSC::Interpreter::tryCachePutByID(class JSC::ExecState *,class JSC::CodeBlock *,struct JSC::Instruction *,class JSC::JSValue,class JSC::PutPropertySlot const &)	C++
 	QtWebKit4.dll!JSC::Interpreter::privateExecute(enum JSC::Interpreter::ExecutionFlag,class JSC::RegisterFile *,class JSC::ExecState *)	C++
 	QtWebKit4.dll!JSC::Interpreter::executeCall(class JSC::ExecState *,class JSC::JSObject *,enum JSC::CallType,union JSC::CallData const &,class JSC::JSValue,class JSC::ArgList const &)	C++
 	QtWebKit4.dll!JSC::call(class JSC::ExecState *,class JSC::JSValue,enum JSC::CallType,union JSC::CallData const &,class JSC::JSValue,class JSC::ArgList const &)	C++
 	QtWebKit4.dll!WebCore::JSMainThreadExecState::call(class JSC::ExecState *,class JSC::JSValue,enum JSC::CallType,union JSC::CallData const &,class JSC::JSValue,class JSC::ArgList const &)	C++
 	QtWebKit4.dll!WebCore::JSEventListener::handleEvent(class WebCore::ScriptExecutionContext *,class WebCore::Event *)	C++
 	QtWebKit4.dll!WebCore::EventTarget::fireEventListeners(class WebCore::Event *,struct WebCore::EventTargetData *,class WTF::Vector<class WebCore::RegisteredEventListener,1> &)	C++
 	QtWebKit4.dll!WebCore::EventTarget::fireEventListeners(class WebCore::Event *)	C++
 	QtWebKit4.dll!WebCore::EventTarget::dispatchEvent(class WTF::PassRefPtr<class WebCore::Event>)	C++
 	QtWebKit4.dll!WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent(class WTF::PassRefPtr<class WebCore::Event>,enum WebCore::ProgressEventAction)	C++
 	QtWebKit4.dll!WebCore::XMLHttpRequest::callReadyStateChangeListener(void)	C++
 	QtWebKit4.dll!WebCore::XMLHttpRequest::didFinishLoading(unsigned long,double)	C++
 	QtWebKit4.dll!WebCore::SubresourceLoader::didFinishLoading(double)	C++

I played around with the code a little and it looks that using a local variable versus an instance attribute was enough to stop the segfault (I also took the opportunity to tidy up the code). It could be something related to pyside 1.2.2 (python 2.7) but I did not have much time/luck into investigating further.

[itziakos]

I am still not sure that this is actually the real problem, but I thought that the changes are generally positive so I submitted the PR

[itziakos]

@prabhuramachandran is this ok to merge?

[prabhuramachandran]

Thanks for all the details! That was helpful. Yes, I think it should be fine to merge. A test case would be nice if possible but I am not sure how hard it would be to create or if it is going to be a bit too much to ask for. I asked because I was just curious about the origin of the PR. Thanks!

[itziakos]

A test case would be nice if possible but I am not sure how hard it would be to create or if it is going to be a bit too much to ask for

Yes, it is tricky. I have not had much luck so far. I will try to get some time and give it another go.

[prabhuramachandran]

@itziakos -- please merge it if the test case is too much of a bother.

[itziakos]

@itziakos -- please merge it if the test case is too much of a bother.

I did not have time look at it yet

[itziakos]

@prabhuramachandran I have not been able to replicate the specific segfault with a small test. However, in the process I found another segfault. The following code will result in a sefault on exit (python 2.7.13 and pyside 1.2.2)

import unittest
import faulthandler

from jigna.qt import QtCore, QtGui
from jigna.core.proxy_qwebview import ProxyQWebView

faulthandler.enable()

class TestProxyQWebview(unittest.TestCase):

    def setUp(self):
        self.app = QtGui.QApplication.instance() or QtGui.QApplication([])

    def test_setUrl(self):
        app = self.app

        def create():
            widget = ProxyQWebView()
            widget.show()
            widget.setUrl(QtCore.QUrl('https://www.enthought.com'))
            print 'DONE'

        QtCore.QTimer.singleShot(10, create)
        QtCore.QTimer.singleShot(10000, app.quit)
        print 'Start'
        app.exec_()

I would expect that just quitting the application should not segfault.

[itziakos]

opened issue #93