xpra v2.2.1-r17715 reproducibly segfaults on mac os high sierra 10.13.2
The problem seems to be near this line http://xpra.org/trac/browser/xpra/trunk/src/xpra/platform/darwin/keyboard.py#L102
At least this simple script sefaults 100% of time:
from AppKit import NSTextInputContext ic = NSTextInputContext.new() print ic del ic
At first I reported it to pyobjc: https://bitbucket.org/ronaldoussoren/pyobjc/issues/235/nstextinputcontext-segfaults-on
But it seems from duscussion of that ticket that the code in xpra is incorrect as NSTextInputContext must be initialized using init method with a parameter.
Thanks for the detailed bug report.
The exact same bug had been reported before: #1725, the workaround we had used was to prevent the context from getting garbage collected.
r18104 should fix this properly this time.
You can find beta python3 / gtk3 2.3 builds with this fix in the macos beta area or you can apply the patch by hand to your installation.
Please close if that works for you.
As far as I understand you either do .new() OR .alloc().init[...] Havent tested yet, but what do you think?
As far as I understand you either do .new() OR .alloc().init[...]
I'm not sure, pyobjc is still a bit of a mistery to me.
I haven't used it, I judge purely by this intro: https://pythonhosted.org/pyobjc/core/intro.html It says that some classes provide .new as a shortcut to alloc().init
The fix seems to be working, thanks. But from documentation lawyer perspective .alloc() is correct, new() is something like alloc() + init() I guess, for example in this snippet
from AppKit import NSTextInputContext, NSTextView #@UnresolvedImport text_input_context = NSTextInputContext.alloc() view = NSTextView.new() text_input_context.initWithClient_(view) del text_input_context
You can change
view = NSTextView.new()
to
view = NSTextView.alloc().init()
but changing it purely to
view = NSTextView.alloc()
also leads to segfault.
I would have thought that initWithClient_
could replace the plain init
call, oh well.
If it no longer segfaults, I think I prefer to leave it as it is.
That's an assumption you for some reason willing to make, ok. Thanks for the fix.
That's an assumption you for some reason willing to make, ok.
I'm not following you.
What assumption am I making here?
You've proved that we have to call either new()
or alloc().init()
(since they're both equivalent) and that's exactly what we do. Where is the assumption?
I see now that you have a call new() and then initWithClient() making an assumption that two calls to init (say the first implicit from new and then explicit initWithClient) are ok. I thought that changing this line https://xpra.org/trac/browser/xpra/trunk/src/xpra/platform/darwin/keyboard.py#L101 to call to alloc() is strictly according to documentation, without additional assumptions.
.. to call to alloc() is strictly according to documentation ...
Yes, it would be more correct. That's what I meant in comment:5.
Since I don't have a 10.13.x system to test on, I would have to rely on you to verify that this works as expected. If you can confirm that this works without crashing, I can make the change.
It works, I think you should replace new with alloc
Done in r18140.
Please let me know if that works, updated beta 2.3 macos python3 build posted here: https://xpra.org/beta/osx/
The latest version doesn't start like this: #1753 The patch from this ticket however works perfectly (applied it to latest stable from homebrew r17715) Thank you for lightning fast reaction and help!
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1750