New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[addons/settings] Fix fallout #2 #12276

Merged
merged 17 commits into from Jun 10, 2017

Conversation

Projects
None yet
3 participants
@Montellese
Member

Montellese commented Jun 9, 2017

These commits should fix the following problems caused by #12125:

  • support string-based labels (not just id-based labels) for categories, groups and settings
  • fix handling of InfoBool conditions for enable and visible
  • treat settings (whose type is not action) as labels (new label control introduced)
  • fix action settings not properly being triggered
  • fix crash when changing settings with a complex setting callback handler
  • support option="close" for action settings

There are still other issues that need fixing which I will cover in separate PRs because they are more complex.

How Has This Been Tested?

Locally with add-ons provided by the people who reported above issues (thanks @basrieter and @primaeval).

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed
@hudokkow

This comment has been minimized.

Show comment
Hide comment
@hudokkow

hudokkow Jun 9, 2017

Member

Crash is no more. Thx!

Member

hudokkow commented Jun 9, 2017

Crash is no more. Thx!

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Jun 9, 2017

Member

@hudokkow thanks for the confirmation. I've added two more fixes (which haven't been reported yet) and am working on the problem with binary add-ons reported by @AlwinEsch.

Member

Montellese commented Jun 9, 2017

@hudokkow thanks for the confirmation. I've added two more fixes (which haven't been reported yet) and am working on the problem with binary add-ons reported by @AlwinEsch.

@Montellese Montellese added this to the L 18.0-alpha1 milestone Jun 10, 2017

@Montellese Montellese merged commit a0a04d6 into xbmc:master Jun 10, 2017

1 check passed

default You're awesome. Have a cookie
Details
@ksooo

This comment has been minimized.

Show comment
Hide comment
@ksooo

ksooo Jun 11, 2017

Member

@Montellese Experiencing a reproducible crash when clicking on the icon of the popular addon "Amazon VOD" (https://github.com/Sandmann79/xbmc/tree/master/plugin.video.amazon-test):

Crash stack trace:

* thread #56, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010068a93d kodi.bin`ADDON::CAddonSettings::Load(this=0x000000012ede74b8, doc=0x000070000e733a20) at AddonSettings.cpp:307
    frame #1: 0x000000010045e736 kodi.bin`ADDON::CAddon::SettingsFromXML(this=0x000000012edb6b30, doc=0x000070000e733a20, loadDefaults=false) at Addon.cpp:344
    frame #2: 0x000000010045a4c1 kodi.bin`ADDON::CAddon::LoadUserSettings(this=0x000000012edb6b30) at Addon.cpp:171
    frame #3: 0x0000000100459ee2 kodi.bin`ADDON::CAddon::LoadSettings(this=0x000000012edb6b30, bForce=false) at Addon.cpp:131
    frame #4: 0x000000010045a98f kodi.bin`ADDON::CAddon::GetSetting(this=0x000000012edb6b30, key="playmethod") at Addon.cpp:212
    frame #5: 0x00000001011ae24b kodi.bin`XBMCAddon::xbmcaddon::Addon::getSetting(this=0x000000012edd79d0, id="playmethod") at Addon.cpp:91
    frame #6: 0x000000010013611e kodi.bin`PythonBindings::xbmcaddon_XBMCAddon_xbmcaddon_Addon_getSetting(self=0x000000012edd71a0, args=0x000000013c5ee210, kwds=0x0000000000000000) at AddonModuleXbmcaddon.i.cpp:200
    frame #7: 0x00000001035ab751 kodi.bin`PyCFunction_Call(func=0x000000013a1c45e0, arg=0x000000013c5ee210, kw=0x0000000000000000) at methodobject.c:85
    frame #8: 0x000000010364ac8c kodi.bin`call_function(pp_stack=0x000070000e734848, oparg=1) at ceval.c:4350
    frame #9: 0x0000000103646e04 kodi.bin`PyEval_EvalFrameEx(f=0x000000012d566620, throwflag=0) at ceval.c:2987
    frame #10: 0x0000000103640f15 kodi.bin`PyEval_EvalCodeEx(co=0x000000012d54c820, globals=0x000000012a007ea0, locals=0x000000012a007ea0, args=0x0000000000000000, argcount=0, kws=0x0000000000000000, kwcount=0, defs=0x0000000000000000, defcount=0, closure=0x0000000000000000) at ceval.c:3582
    frame #11: 0x000000010363fe25 kodi.bin`PyEval_EvalCode(co=0x000000012d54c820, globals=0x000000012a007ea0, locals=0x000000012a007ea0) at ceval.c:669
    frame #12: 0x000000010367c972 kodi.bin`run_mod(mod=0x000000010f5cc7c0, filename="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", globals=0x000000012a007ea0, locals=0x000000012a007ea0, flags=0x0000000000000000, arena=0x000000013c510920) at pythonrun.c:1376
    frame #13: 0x000000010367cd4f kodi.bin`PyRun_FileExFlags(fp=0x000000010a8f62b0, filename="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", start=257, globals=0x000000012a007ea0, locals=0x000000012a007ea0, closeit=1, flags=0x0000000000000000) at pythonrun.c:1362
    frame #14: 0x0000000101242629 kodi.bin`CPythonInvoker::executeScript(this=0x00000001312607f0, fp=0x000000010a8f62b0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", module=0x000000012c9a2af0, moduleDict=0x000000012a007ea0) at PythonInvoker.cpp:442
    frame #15: 0x000000010123dc17 kodi.bin`CPythonInvoker::execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at PythonInvoker.cpp:300
    frame #16: 0x0000000101059e03 kodi.bin`ILanguageInvoker::Execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at ILanguageInvoker.cpp:41
    frame #17: 0x000000010123b16d kodi.bin`CPythonInvoker::Execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at PythonInvoker.cpp:154
    frame #18: 0x000000010105aacc kodi.bin`CLanguageInvokerThread::Process(this=0x000000012d4ba770) at LanguageInvokerThread.cpp:91
    frame #19: 0x000000010105aaf9 kodi.bin`non-virtual thunk to CLanguageInvokerThread::Process(this=0x000000012d4ba770) at LanguageInvokerThread.cpp:0
    frame #20: 0x0000000101c400b0 kodi.bin`CThread::Action(this=0x000000012d4ba770) at Thread.cpp:221
    frame #21: 0x0000000101c3e729 kodi.bin`CThread::staticThread(data=0x000000012d4ba770) at Thread.cpp:131
    frame #22: 0x0000000107b90b90 libsystem_pthread.dylib`_pthread_body + 180
    frame #23: 0x0000000107b90adc libsystem_pthread.dylib`_pthread_start + 286
    frame #24: 0x0000000107b902c5 libsystem_pthread.dylib`thread_start + 13

Crashing line (newSetting is nullptr):

if (!newSetting->FromString(setting.second))

Log excerpt:

2017-06-11 20:37:48.906988+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: cannot reference setting (relative index: -100; absolute index: -95) in another category in old setting condition "eq(-100,true)" for "login_acc"
2017-06-11 20:37:48.907020+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: failed to parse enable condition "eq(-100,true)" of old setting definition for "login_acc"
2017-06-11 20:37:48.907066+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: cannot reference setting (relative index: -100; absolute index: -94) in another category in old setting condition "eq(-100,true)" for "use_mfa"
2017-06-11 20:37:48.907097+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: failed to parse visible condition "eq(-100,true)" of old setting definition for "use_mfa"
Member

ksooo commented Jun 11, 2017

@Montellese Experiencing a reproducible crash when clicking on the icon of the popular addon "Amazon VOD" (https://github.com/Sandmann79/xbmc/tree/master/plugin.video.amazon-test):

Crash stack trace:

* thread #56, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010068a93d kodi.bin`ADDON::CAddonSettings::Load(this=0x000000012ede74b8, doc=0x000070000e733a20) at AddonSettings.cpp:307
    frame #1: 0x000000010045e736 kodi.bin`ADDON::CAddon::SettingsFromXML(this=0x000000012edb6b30, doc=0x000070000e733a20, loadDefaults=false) at Addon.cpp:344
    frame #2: 0x000000010045a4c1 kodi.bin`ADDON::CAddon::LoadUserSettings(this=0x000000012edb6b30) at Addon.cpp:171
    frame #3: 0x0000000100459ee2 kodi.bin`ADDON::CAddon::LoadSettings(this=0x000000012edb6b30, bForce=false) at Addon.cpp:131
    frame #4: 0x000000010045a98f kodi.bin`ADDON::CAddon::GetSetting(this=0x000000012edb6b30, key="playmethod") at Addon.cpp:212
    frame #5: 0x00000001011ae24b kodi.bin`XBMCAddon::xbmcaddon::Addon::getSetting(this=0x000000012edd79d0, id="playmethod") at Addon.cpp:91
    frame #6: 0x000000010013611e kodi.bin`PythonBindings::xbmcaddon_XBMCAddon_xbmcaddon_Addon_getSetting(self=0x000000012edd71a0, args=0x000000013c5ee210, kwds=0x0000000000000000) at AddonModuleXbmcaddon.i.cpp:200
    frame #7: 0x00000001035ab751 kodi.bin`PyCFunction_Call(func=0x000000013a1c45e0, arg=0x000000013c5ee210, kw=0x0000000000000000) at methodobject.c:85
    frame #8: 0x000000010364ac8c kodi.bin`call_function(pp_stack=0x000070000e734848, oparg=1) at ceval.c:4350
    frame #9: 0x0000000103646e04 kodi.bin`PyEval_EvalFrameEx(f=0x000000012d566620, throwflag=0) at ceval.c:2987
    frame #10: 0x0000000103640f15 kodi.bin`PyEval_EvalCodeEx(co=0x000000012d54c820, globals=0x000000012a007ea0, locals=0x000000012a007ea0, args=0x0000000000000000, argcount=0, kws=0x0000000000000000, kwcount=0, defs=0x0000000000000000, defcount=0, closure=0x0000000000000000) at ceval.c:3582
    frame #11: 0x000000010363fe25 kodi.bin`PyEval_EvalCode(co=0x000000012d54c820, globals=0x000000012a007ea0, locals=0x000000012a007ea0) at ceval.c:669
    frame #12: 0x000000010367c972 kodi.bin`run_mod(mod=0x000000010f5cc7c0, filename="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", globals=0x000000012a007ea0, locals=0x000000012a007ea0, flags=0x0000000000000000, arena=0x000000013c510920) at pythonrun.c:1376
    frame #13: 0x000000010367cd4f kodi.bin`PyRun_FileExFlags(fp=0x000000010a8f62b0, filename="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", start=257, globals=0x000000012a007ea0, locals=0x000000012a007ea0, closeit=1, flags=0x0000000000000000) at pythonrun.c:1362
    frame #14: 0x0000000101242629 kodi.bin`CPythonInvoker::executeScript(this=0x00000001312607f0, fp=0x000000010a8f62b0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", module=0x000000012c9a2af0, moduleDict=0x000000012a007ea0) at PythonInvoker.cpp:442
    frame #15: 0x000000010123dc17 kodi.bin`CPythonInvoker::execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at PythonInvoker.cpp:300
    frame #16: 0x0000000101059e03 kodi.bin`ILanguageInvoker::Execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at ILanguageInvoker.cpp:41
    frame #17: 0x000000010123b16d kodi.bin`CPythonInvoker::Execute(this=0x00000001312607f0, script="/Users/kai/Library/Application Support/Kodi/addons/plugin.video.amazon-test/default.py", arguments=size=3) at PythonInvoker.cpp:154
    frame #18: 0x000000010105aacc kodi.bin`CLanguageInvokerThread::Process(this=0x000000012d4ba770) at LanguageInvokerThread.cpp:91
    frame #19: 0x000000010105aaf9 kodi.bin`non-virtual thunk to CLanguageInvokerThread::Process(this=0x000000012d4ba770) at LanguageInvokerThread.cpp:0
    frame #20: 0x0000000101c400b0 kodi.bin`CThread::Action(this=0x000000012d4ba770) at Thread.cpp:221
    frame #21: 0x0000000101c3e729 kodi.bin`CThread::staticThread(data=0x000000012d4ba770) at Thread.cpp:131
    frame #22: 0x0000000107b90b90 libsystem_pthread.dylib`_pthread_body + 180
    frame #23: 0x0000000107b90adc libsystem_pthread.dylib`_pthread_start + 286
    frame #24: 0x0000000107b902c5 libsystem_pthread.dylib`thread_start + 13

Crashing line (newSetting is nullptr):

if (!newSetting->FromString(setting.second))

Log excerpt:

2017-06-11 20:37:48.906988+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: cannot reference setting (relative index: -100; absolute index: -95) in another category in old setting condition "eq(-100,true)" for "login_acc"
2017-06-11 20:37:48.907020+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: failed to parse enable condition "eq(-100,true)" of old setting definition for "login_acc"
2017-06-11 20:37:48.907066+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: cannot reference setting (relative index: -100; absolute index: -94) in another category in old setting condition "eq(-100,true)" for "use_mfa"
2017-06-11 20:37:48.907097+0200 kodi.bin[94610:2717489] Debug Print: CAddonSettings[plugin.video.amazon-test]: failed to parse visible condition "eq(-100,true)" of old setting definition for "use_mfa"

@Montellese Montellese deleted the Montellese:addon_settings_fallout_2 branch Oct 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment