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

Fix crash on exit. #13445

Merged
merged 1 commit into from Jan 27, 2018
Merged

Fix crash on exit. #13445

merged 1 commit into from Jan 27, 2018

Conversation

@jimfcarroll
Copy link
Member

jimfcarroll commented Jan 26, 2018

This crash seems to be due to the logger being set on CThreads (and Exception) using XbmcContext rather than the means prescribed by GlobalHandling.

Description

XbmcContext was originally supposed to be the "down payment" on a dependency injection approach to modularizing the code but it never went anywhere so it's now removed all together. All of the code that was dependent on this functionality has been reverted to the standard means.

Motivation and Context

Crash on exit.

How Has This Been Tested?

Ran it several time on Linux so far to make sure it doesn't crash. Since the crash was rare this isn't a foolproof means of testing. @FernetMenta was able to reproduce it relatively easily.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • 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
@jimfcarroll jimfcarroll force-pushed the jimfcarroll:crash-on-exit branch from 75d0902 to a1dbfdd Jan 26, 2018
@FernetMenta

This comment has been minimized.

Copy link
Member

FernetMenta commented Jan 27, 2018

thanks!
+25 −164
:)


void Exception::LogThrowMessage(const char* prefix) const
{
CLog::Log(LOGERROR,"EXCEPTION Thrown (%s) : %s", classname.c_str(), message.c_str());

This comment has been minimized.

Copy link
@Rechi

Rechi Jan 27, 2018

Member

indentation

This comment has been minimized.

Copy link
@jimfcarroll

jimfcarroll Jan 27, 2018

Author Member

hummm. That's odd. I thought I had that right. Will fix.

@@ -88,7 +87,6 @@ void XBMC_POSIX_HandleSignal(int sig)
int main(int argc, char* argv[])
{
// set up some xbmc specific relationships

This comment has been minimized.

Copy link
@Rechi

Rechi Jan 27, 2018

Member

comment still valid?

@@ -127,7 +126,6 @@ INT WINAPI WinMain(HINSTANCE hInst, HINSTANCE, LPSTR commandLine, INT)
int status;
{
// set up some xbmc specific relationships

This comment has been minimized.

Copy link
@Rechi

Rechi Jan 27, 2018

Member

comment still valid?

@@ -226,7 +223,7 @@ void CThread::Action()
}
catch (...)
{
LOG(LOGERROR, "%s - thread %s, Unhandled exception caught in thread process, aborting. auto delete: %d", __FUNCTION__, m_ThreadName.c_str(), IsAutoDelete());
CLog::Log(LOGERROR, "%s - thread %s, Unhandled exception caught in thread process, aborting. auto delete: %d", __FUNCTION__, m_ThreadName.c_str(), IsAutoDelete());

This comment has been minimized.

Copy link
@Rechi

Rechi Jan 27, 2018

Member

indentation

@jimfcarroll

This comment has been minimized.

Copy link
Member Author

jimfcarroll commented Jan 27, 2018

Okay, Dealt with the issues that @Rechi pointed out.

… on CThreads (and Exception) using XbmcContext rather than the means prescribed by GlobalHandling. XbmcContext was originally supposed to be the "down payment" on a dependency injection approach to modularizing the code but it never went anywhere so it's now removed all together.
@jimfcarroll jimfcarroll force-pushed the jimfcarroll:crash-on-exit branch from 48c1d16 to d765a72 Jan 27, 2018
@jimfcarroll

This comment has been minimized.

Copy link
Member Author

jimfcarroll commented Jan 27, 2018

Commits now squashed

@jimfcarroll jimfcarroll reopened this Jan 27, 2018
@jimfcarroll jimfcarroll merged commit 4798f3e into xbmc:master Jan 27, 2018
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@jimfcarroll jimfcarroll deleted the jimfcarroll:crash-on-exit branch Jan 27, 2018
@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Jan 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.