Xpra: Ticket #1750: Segfault on mac os 10.13.2

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.

Mon, 22 Jan 2018 12:58:56 GMT - Antoine Martin: owner, description changed

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.

Tue, 23 Jan 2018 10:31:45 GMT - unkulunkulu:

As far as I understand you either do .new() OR .alloc().init[...] Havent tested yet, but what do you think?

Tue, 23 Jan 2018 11:09:34 GMT - Antoine Martin:

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.

Tue, 23 Jan 2018 13:15:18 GMT - unkulunkulu:

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()
del text_input_context

You can change

view = NSTextView.new()


view = NSTextView.alloc().init()

but changing it purely to

view = NSTextView.alloc()

also leads to segfault.

Tue, 23 Jan 2018 14:10:41 GMT - Antoine Martin: status changed; resolution set

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.

Tue, 23 Jan 2018 14:18:40 GMT - unkulunkulu:

That's an assumption you for some reason willing to make, ok. Thanks for the fix.

Tue, 23 Jan 2018 14:25:16 GMT - Antoine Martin:

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?

Tue, 23 Jan 2018 15:01:25 GMT - unkulunkulu:

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.

Wed, 24 Jan 2018 03:58:45 GMT - Antoine Martin:

.. 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.

Wed, 24 Jan 2018 13:04:21 GMT - unkulunkulu:

It works, I think you should replace new with alloc

Wed, 24 Jan 2018 14:17:26 GMT - Antoine Martin:

Done in r18140.

Please let me know if that works, updated beta 2.3 macos python3 build posted here: https://xpra.org/beta/osx/

Thu, 25 Jan 2018 11:20:21 GMT - unkulunkulu:

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!

Sat, 23 Jan 2021 05:32:48 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1750