Skip to content
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 destructor visibility to allow polymorphic deletion #5338

Merged
merged 1 commit into from Sep 8, 2014

Conversation

xhaggi
Copy link
Member

@xhaggi xhaggi commented Sep 8, 2014

This is a follow-up of #5334 which fixes the root cause of @ace20022's issue.

@opdenkamp mind taking a look .. if fine i will close the other PR.

This fixes the polymorphic deletion in CGUIWindowPVRBase.
@xhaggi
Copy link
Member Author

xhaggi commented Sep 8, 2014

jenkins build this please

@opdenkamp
Copy link
Member

yup, that'll fix it. thanks @FernetMenta for spotting it :)

opdenkamp pushed a commit that referenced this pull request Sep 8, 2014
[pvr] fix destructor visibility to allow polymorphic deletion
@opdenkamp opdenkamp merged commit 15525e0 into xbmc:master Sep 8, 2014
@ace20022
Copy link
Member

ace20022 commented Sep 9, 2014

Can someone explain in the context of the orig bug?

I can't, but no one seems to be interested in argument. I said several times that it didn't solve my issue.

One thing I noticed: the crash only happens when I close kodi by exit, but not by shutdown nor by closing the whole window. In the latter cases the dtors are called in another sequence, i.e., CGUIInfoManager is deleted before GUIWindowPVRChannels.

@FernetMenta
Copy link
Contributor

dtors are called in order of direction to the base class if virtual. assume you have a protected dtor and delete this object. the protected dtor won't be called. now we have
C public virtaul dtor
B protected virtual dtor
A public virtual dtor

our hypothesis was that when object C is destructed, the protected dtor of B breaks the chain so that A won't be called either.

@ace20022
Copy link
Member

ace20022 commented Sep 9, 2014

But the problem is that the object is destructed and not that it is not!

@FernetMenta
Copy link
Contributor

the dtor of class Observer does already what you added to the dtor of C in your PR. it is supposed to unregister the observer from all observables. if this does not work something other is wrong and needs to be investigated.

@ace20022
Copy link
Member

ace20022 commented Sep 9, 2014

I've described all of my observations in the second last comment. Imo it is the order of destructing objects and I guess no one can enforce the correct order (or prevent that some dev breaks that order in the future) and I therefore proposed that fix. But of course I could be wrong.

@FernetMenta
Copy link
Contributor

if the order of destruction plays a role, something goes wrong I guess. what are the steps to make it crash?

the crash only happens when I close kodi by exit

does not happen here when I click the quit button.

@ace20022
Copy link
Member

ace20022 commented Sep 9, 2014

do you have time to chat (irc)?

@ace20022
Copy link
Member

ace20022 commented Sep 9, 2014

I'm using the vdr plugin with the hash 3949502043d520e4707bc3109abf3c6edb23cf67.
I can see a different order between the quit/exit methods in vs with breakspoints set as follows:
https://github.com/xbmc/xbmc/blob/master/xbmc/pvr/windows/GUIWindowPVRBase.cpp#L63
https://github.com/xbmc/xbmc/blob/master/xbmc/GUIInfoManager.cpp#L135

@xhaggi xhaggi deleted the pvr-fix-destructor-visibility branch September 14, 2014 07:39
@MartijnKaijser MartijnKaijser added this to the Helix 14.0-alpha4 milestone Sep 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants