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

[PVR] Fix trac17246 - another approach #11615

Merged
merged 4 commits into from Feb 6, 2017

Conversation

Projects
None yet
2 participants
@ksooo
Member

ksooo commented Feb 4, 2017

This is a fix for trac 17246: http://trac.kodi.tv/ticket/17246

Completely new approach this time, which should no longer lead to crashes the previous approach has caused. ;-)

The gist of the fix is to obtain and remember the pointer to the extended progress bar dialog instance early (PVR Manager init), avoiding the deadly g_WindowManager.GetWindow() call.

I discovered other scenarios which could lead to the same deadlock (in CPVRMananger::ShowProgressDialog(string,int))which are are also fixed now.

The stack trace of one of the possible deadlocks:
kodi-frozen

@Jalle19 for review?

@ksooo ksooo added this to the L 18.0-alpha1 milestone Feb 4, 2017

@ksooo ksooo requested a review from Jalle19 Feb 4, 2017

@Jalle19

If it works, sure. I'm wondering if it would make sense to remove this feature altogether since most backends AFAIK are able to supply channel icons on their own.

@ksooo

This comment has been minimized.

Show comment
Hide comment
@ksooo

ksooo Feb 5, 2017

Member

jenkins build this please

Member

ksooo commented Feb 5, 2017

jenkins build this please

@ksooo

This comment has been minimized.

Show comment
Hide comment
@ksooo

ksooo Feb 5, 2017

Member

We have positive feedback from the reporter of the crashes the first approach had introduced: http://forum.kodi.tv/showthread.php?tid=298461&pid=2516755#pid2516755

Member

ksooo commented Feb 5, 2017

We have positive feedback from the reporter of the crashes the first approach had introduced: http://forum.kodi.tv/showthread.php?tid=298461&pid=2516755#pid2516755

@ksooo

This comment has been minimized.

Show comment
Hide comment
@ksooo

ksooo Feb 6, 2017

Member

The submitter of the trac ticket got a a test build, did runtime testing, and confirmed that the original issue is solved, no crashes.

Member

ksooo commented Feb 6, 2017

The submitter of the trac ticket got a a test build, did runtime testing, and confirmed that the original issue is solved, no crashes.

@ksooo ksooo merged commit 7dfdd90 into xbmc:master Feb 6, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins4kodi You did a great job. Have a cookie.
Details

@ksooo ksooo deleted the ksooo:fix-trac17246-v2 branch Feb 6, 2017

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