Segfault on mac os 10.13.2

[unkulunkulu]

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.

[Antoine Martin]

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.

[unkulunkulu]

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

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

[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()
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.

[Antoine Martin]

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.

[unkulunkulu]

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

[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?

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

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

[unkulunkulu]

It works, I think you should replace new with alloc

[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/

[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!