Bug 154902 - Phonon Xine-backend crashes on KDE-logout
Summary: Phonon Xine-backend crashes on KDE-logout
Status: RESOLVED WORKSFORME
Alias: None
Product: Phonon
Classification: Frameworks and Libraries
Component: Xine backend (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Matthias Kretz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-31 15:11 UTC by Anne-Marie Mahfouf
Modified: 2008-06-02 17:17 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
backtrace of the phonon/xine crash (7.12 KB, text/plain)
2008-01-03 04:44 UTC, Sebastian Sauer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anne-Marie Mahfouf 2007-12-31 15:11:06 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources
OS:                Linux

we (cheko on IRC #plasma reported the bug first) found so far 2 settings that are reset at logout:
- VisibleItemsCount for the Number of Visible Items in Kickoff
- Show tooltips for the Task Manager

The file plasma-appletsrc is overwritten at logout. Settings are kept if I killall plasma though.
Comment 1 Gabriel C 2007-12-31 15:27:30 UTC
can confirm that and more :) all settings are overwritten at logout , clock etc etc
Comment 2 Luka Renko 2007-12-31 16:04:14 UTC
The same is true for Battery applet if it is placed on the panel. Options are lost on logout.
Comment 3 Anne-Marie Mahfouf 2007-12-31 16:11:28 UTC
Latest: all panel settings are lost on logout. And the good news is: it is a recent bug, it worked some days ago!
Comment 4 Anne-Marie Mahfouf 2007-12-31 16:15:32 UTC
When you kill X with init 3 for example (kill X without using logout) then the settings are kept so the faulty code in probably in the logout code
Comment 5 Aaron J. Seigo 2008-01-01 21:19:07 UTC
this really sounds like plasma is just being killed rather than actually quit. settings are certainly saved here when plasma is quit, so if desktop shutdown is not properly causing plasma to exit ...... don't know what to do about that really. that will always leave the possibility open for on-quit code not to be run that results in poor behaviour.
Comment 6 Luka Renko 2008-01-01 21:57:59 UTC
It is very consistent here (SVN from today) and I can reproduce it even with kquitapp (which I think should cause proper shutdown of plasma). My test case to reproduce:

1. kquitapp plasma    # kill plasma started by session
2. plasma
3. Set kickoff option to 15 lines
4. kquitapp plasma
5. plasma
6. kickoff uses default size of 10 lines 
Comment 7 Sebastian Sauer 2008-01-01 22:35:26 UTC
quit interesting taking into account that the kickoff-applet does;

KConfigGroup cg = config();
cg.writeEntry("SwitchTabsOnHover",d->switchTabsOnHover);
cg.writeEntry("VisibleItemsCount",d->visibleItemsCount);
cg.config()->sync();

so, it sync's the KConfig if changed what means, the change should be direct written to the kconfig-backend.

Future testing shows, that the value is written correct
> cat ~/.kde4/share/config/plasma-appletsrc |grep VisibleItemsCount
> VisibleItemsCount=20

and sometimes even it stays in there, _but_ mostly it seems something crashes on logout and then the already written item got lost (probably a roolback or something like this?). Also if it crashes, the crashhandler pop's up to just quickly disappear again what prevents to catch the backtrace and sometimes that happens even such fast that the crashhandler-dialog isn't really visible at all.
Comment 8 Sebastian Sauer 2008-01-01 22:59:27 UTC
The console output indicates that it's phonon/knotify that crashes; 
 
 ksmserver(12056)/kdeui (KNotification) KNotificationManager::notificationClosed: 6 
 ksmserver(12056) KSMServer::logoutSoundFinished: Logout event finished 
 ksmserver(12056) KSMServer::startKilling: Starting killing clients 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "kwinsmhelper" ( 10cd617274000119922427000000120560000 ) 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "knotify4" ( 10cd617274000119922427100000120560001 ) 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "krunner" ( 10cd617274000119922427100000120560002 ) 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "plasma" ( 10cd617274000119922427100000120560003 ) 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "klipper" ( 10cd617274000119922427300000120560004 ) 
 ksmserver(12056) KSMServer::startKilling: completeShutdown: client  "kaccess" ( 10cd617274000119922427300000120560005 ) 
 ksmserver(12056) KSMServer::startKilling:  We killed all clients. We have now clients.count()= 7 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 7 
 ksmserver(12056)/kdeui (KNotification) KNotification::~KNotification: 6 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::~XineEngine: delete Phonon::Xine::AudioPortDeleter 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::~XineEngine: delete QObject 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 6 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 5 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::xineEventListener: XINE_EVENT_QUIT 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::xineEventListener: XINE_EVENT_QUIT 
 plasma(12067)/kio (KDirLister) KDirLister::stop: 
 plasma(12067)/kio (KDirListerCache) KDirListerCache::forgetDirs: KDirLister(0x779da8)  item moved into cache:  KUrl("file:///home/kde4/Desktop") 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 4 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 3 
 plasma(12067) Plasma::LayoutItem::setLayout: layout removed from under us. expect crashes 
 plasma(12067) Plasma::LayoutItem::setLayout: layout removed from under us. expect crashes 
 plasma(12067) Plasma::LayoutItem::setLayout: layout removed from under us. expect crashes 
 plasma(12067) Plasma::LayoutItem::setLayout: layout removed from under us. expect crashes 
 plasma(12067) Plasma::LayoutItem::setLayout: layout removed from under us. expect crashes 
 plasma(12067)/kio (KDirListerCache) KDirListerCache::~KDirListerCache: -KDirListerCache 
 ksmserver(12056) KSMServer::completeKilling: KSMServer::completeKilling clients.count()= 2 
 knotify(12064)/phonon (xine backend) Phonon::Xine::AudioPortData::~AudioPortData: ----------------------------------------------- audio_port destroyed 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::~XineEngine: delete Phonon::Xine::AudioPortDeleter 
 knotify(12064)/phonon (xine backend) Phonon::Xine::XineEngine::~XineEngine: delete QObject 
 KCrash: crashing... crashRecursionCounter = 2 
 KCrash: Application Name = knotify4 path = <unknown> pid = 12064 
 sock_file=/home/kde4/.kde4/socket-earth/kdeinit4__3 
 
Interesting here that we have 2 times the "Phonon::Xine::XineEngine::~XineEngine" ... seems here to disable the sound for the logout-notification fixes the crash for me.
Comment 9 Aaron J. Seigo 2008-01-01 23:09:30 UTC
@Sebas: great work on tracking this one down. so it isn't a plasma bug either =/ this is one of the things i'm not looking forward to in 4.0: every problem in every stack every person encounters that isn't obviously an application is going to get filed under plasma. plasma will become the bug triage center for so much framework stuff =(

anyways ... 

kickoff should never be calling sync() on the config file. we need a generic mechanism for applets to signify that they have config that needs to be saved and plasma itself needs to schedule the sync. otherwise we will end up with much more disk activity than we should ever have, just as we did with kicker.

anyways, i'm going to reassign this to the phonon component.
Comment 10 Sebastian Sauer 2008-01-01 23:25:49 UTC
re my comment #8 "seems here to disable the sound for the logout-notification fixes the crash for me. "
Seems I was to fast there. So, good news are that if Phonon got complete disabled, dr. konqi doesn't show up any longer _but_ (the bad news); comment #6 is still reproducable :-(

[...]
ksmserver(13664) KSMServer::completeKillingWM: KSMServer::completeKillingWM clients.count()= 0
kdeinit4: PID 13669 terminated.
startkde: Shutting down...
kdeinit4: terminate KDE.
klauncher: Exiting on signal 1
kded(13657)/kdecore (KConfigSkeleton) KCoreConfigSkeleton::writeConfig: KCoreConfigSkeleton::writeConfig()
kded(13657)/kdecore (KConfigSkeleton) KCoreConfigSkeleton::readConfig: KCoreConfigSkeleton::readConfig()
startkde: Running shutdown scripts...
startkde: Done.
Comment 11 Sebastian Sauer 2008-01-02 00:29:14 UTC
okeli, after hours of testing following patch finally "fixes" the problem. That means, once that patch got applied to kdebase/workspace/libs/plasma the content of the config-file doesn't got lost any longer. For sure it's not perfect since it just disables the probably needed deleteGroup() but at least it shows where the problem seems to be located...

Index: corona.cpp
===================================================================
--- corona.cpp  (revision 755434)
+++ corona.cpp  (working copy)
@@ -138,7 +138,7 @@
         containment->saveConstraints(&containmentConfig);
         containment->save(&containmentConfig);
         KConfigGroup applets(&containmentConfig, "Applets");
-        applets.deleteGroup();
+        //applets.deleteGroup();
         foreach (const Applet* applet, containment->applets()) {
             KConfigGroup appletConfig(&applets, QString::number(applet->id()));
             applet->save(&appletConfig);
Comment 12 Sebastian Sauer 2008-01-02 00:32:47 UTC
added panel-devel@kde.org as CC since it seems this report does contain one case related to phonon and one case related to Plasma.

@Aaron could you please have a look at the patch above aka maybe that helps to come up with a solution? If not, I'll try to investigate it tomorrow more detailed. Thx :)
Comment 13 Aaron J. Seigo 2008-01-02 00:48:01 UTC
@Sebastian: i already fixed the plasma issue. it was caused by applet authors never implementing saveState in a sensible manner and Corona assuming they would. oh well. it's fixed now.
Comment 14 Sebastian Sauer 2008-01-02 00:57:06 UTC
great, thanks :-)

so, next step (phonon/knotify) would be then imho to either fix the crash rather fast or just disable sounds for notifications that happen on shutdown like the logout or the window closed sounds. t-9 days till release, time is running :)
Comment 15 Sebastian Sauer 2008-01-03 04:44:38 UTC
Created attachment 22817 [details]
backtrace of the phonon/xine crash

Attached is a backtrace I was finally able to catch of the xine/phonon crash on
logout that is here 100% reproducable on each logout.
Comment 16 Sebastian Sauer 2008-01-03 04:46:55 UTC
* remove panel-devel from the CC list
* changed topic to show what the report is about (creating a new report would be an option too but it seems that's not possible atm cause of timeouts).
Comment 17 Matthias Kretz 2008-06-02 17:16:25 UTC
Is this still reproduceable for anyone? I've not seen a problem with xine shutdown for a long time now.
Comment 18 Matthias Kretz 2008-06-02 17:17:24 UTC
*** Bug 161572 has been marked as a duplicate of this bug. ***