#1750 closed defect (fixed)
Segfault on mac os 10.13.2
Reported by: | unkulunkulu | Owned by: | unkulunkulu |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | android | Version: | 2.2.x |
Keywords: | Cc: |
Description (last modified by )
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.
Change History (13)
comment:1 Changed 3 years ago by
Description: | modified (diff) |
---|---|
Owner: | changed from Antoine Martin to unkulunkulu |
comment:2 Changed 3 years ago by
As far as I understand you either do .new() OR .alloc().init[...]
Havent tested yet, but what do you think?
comment:3 Changed 3 years ago by
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.
comment:4 Changed 3 years ago by
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.
comment:5 Changed 3 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
comment:6 Changed 3 years ago by
That's an assumption you for some reason willing to make, ok. Thanks for the fix.
comment:7 Changed 3 years ago by
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?
comment:8 Changed 3 years ago by
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.
comment:9 Changed 3 years ago by
.. 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.
comment:11 Changed 3 years ago by
Done in r18140.
Please let me know if that works, updated beta 2.3 macos python3 build posted here: https://xpra.org/beta/osx/
comment:12 Changed 3 years ago by
comment:13 Changed 6 weeks ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1750
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.