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

fixed: race condition when starting a new fresh profile besides master w... #1993

Merged
merged 1 commit into from Dec 29, 2012
Merged

fixed: race condition when starting a new fresh profile besides master w... #1993

merged 1 commit into from Dec 29, 2012

Conversation

wsoltys
Copy link

@wsoltys wsoltys commented Dec 27, 2012

...hich leads to an abort in cpluff. CAddonInstaller::UpdateRepos is called at startup and still runs if you login and CGUIWindowLoginScreen::LoadProfile calls ADDON::CAddonMgr::Get().ReInit() which leads to an invalid cpluff handle for the update job and aborts XBMC (#13826).

I created a pr as I dunno if this is the right way to fix it and if it could have other side effects. For me it runs fine and the update job is triggered again if the second profile is loaded.

@wsoltys
Copy link
Author

wsoltys commented Dec 27, 2012

Logfile for the abort: http://www.xbmclogs.com/show.php?id=24354

@jmarshallnz
Copy link
Contributor

Try wrapping anything that uses m_cpluff and m_cp_context with a lock on the critical section.

@wsoltys
Copy link
Author

wsoltys commented Dec 27, 2012

Not sure if this is enough. CAddonInstaller and CAddonMgr have its own critsection and the update job runs in the CRepositoryUpdateJob context. Another point is that CRepositoryUpdateJob works with the database which at this point is not existing for that new profile. It'll create when the profile is loaded, the update jobs runs when the login screen is shown.

@wsoltys
Copy link
Author

wsoltys commented Dec 29, 2012

I wrapped m_cpluff in AddonManager with the critsect. Now XBMC won't abort any more but as stated above the database accesses still fail http://www.xbmclogs.com/show.php?id=24660.
IMO we shouldn't run the update job when showing the login screen as at this point we dunno which profile will be chosen and therefore for which profile the job should run. Especially as we run the job again when we have logged into the new profile.
What I don't know is if the way I have implemented it here is the right way and on the right spot.

@jmarshallnz
Copy link
Contributor

Ok, add a comment such as

// don't run repo update jobs while on the login screen which runs under the master profile

and pull it in.

The critical section stuff should be a separate PR - I suggest merging that one after Frodo.

…r which leads to an abort in cpluff. CAddonInstaller::UpdateRepos is called at startup and still runs if you login and CGUIWindowLoginScreen::LoadProfile calls ADDON::CAddonMgr::Get().ReInit() which leads to an invalid cpluff handle for the update job and aborts XBMC (#13826)
wsoltys pushed a commit that referenced this pull request Dec 29, 2012
fixed: race condition when starting a new fresh profile besides master.
@wsoltys wsoltys merged commit 3a3e082 into xbmc:master Dec 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants