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

EpgContainer: Fix Deadlock when calling Observer with lock #4751

Merged
merged 1 commit into from
May 20, 2014

Conversation

fritsch
Copy link
Member

@fritsch fritsch commented May 19, 2014

Quick fix especially for v13. Longterm should use proper async handling.

Edit: This is also relevant for master of course.

@fritsch
Copy link
Member Author

fritsch commented May 19, 2014

jenkins build this please

@fritsch
Copy link
Member Author

fritsch commented May 19, 2014

Backtrace: http://paste.ubuntu.com/7479831/

@fritsch fritsch added the Gotham label May 19, 2014
@MartijnKaijser MartijnKaijser added this to the Helix 14.0-alpha1 milestone May 19, 2014
@t-nelson
Copy link
Contributor

Maybe do this against Gotham instead? Otherwise we probably pull our normal shit and not do the right thing later for master since there's no incentive.

@fritsch
Copy link
Member Author

fritsch commented May 19, 2014

It's a bug in master, too. So a non deadlocking master is highly appreciated until @opdenkamp is less busy.

Edit: s/segfault/deadlock/

@jmarshallnz
Copy link
Contributor

Ah, the famous "quick fix", huh? :)

What was the goal here? To get the notification outside the critical section? If so, there's actually a double critical section usage here (to be sure, to be sure?) See ln 673.

@fritsch
Copy link
Member Author

fritsch commented May 19, 2014

Yes - you are perfectly right. That was in deed a quick fix and I simply overlooked that other section. The "fix" does not make any sense now. I will close that PR and leave it to the experts.

@fritsch fritsch closed this May 19, 2014
@jmarshallnz
Copy link
Contributor

I think you can just drop the inside critical section (or the outside one if doing a premature update instead of correctly interpreting m_iNextEpgActiveTagCheck isn't an issue).

Then unlock the crit section prior to your current change.

@opdenkamp comments?

@fritsch fritsch reopened this May 19, 2014
@FernetMenta
Copy link
Contributor

I think you can safely move the other block too. the good thing about bReturn is that it always exits the methond at its end.

@fritsch
Copy link
Member Author

fritsch commented May 19, 2014

jenkins build this please

@opdenkamp
Copy link
Member

the diff is annoying to read like this on github, but looks okay :)

@t-nelson
Copy link
Contributor

@opdenkamp fritsch@820b852?w=0

?w=0 is your friend :)

@opdenkamp
Copy link
Member

oh wow, why isn't that enabled by default? thanks! :)

@jmarshallnz
Copy link
Contributor

That is a neat trick :)

jmarshallnz added a commit that referenced this pull request May 20, 2014
EpgContainer: Fix Deadlock when calling Observer with lock
@jmarshallnz jmarshallnz merged commit 13eea1a into xbmc:master May 20, 2014
@t-nelson t-nelson removed the Gotham label Jun 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants