Bug 261538 - KXMLGUIClient memory corruption warning
Summary: KXMLGUIClient memory corruption warning
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 0.15.0
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
: 271052 283942 288650 295005 295358 305726 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-29 15:14 UTC by Tommi Tervo
Modified: 2019-03-26 14:57 UTC (History)
20 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.9.1
Sentry Crash Report:


Attachments
VG log (20.08 KB, application/x-bzip)
2010-12-29 15:14 UTC, Tommi Tervo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tommi Tervo 2010-12-29 15:14:39 UTC
Created attachment 55349 [details]
VG log

Version:           0.5.81 (using KDE 4.5.90) 
OS:                Linux

Hello

Probably known problem but I couldn't find this warning on quick bz search.

Error: Expected the optional content group list, but wasn't able to find it, or it isn't an Array
okular(22218)/kdeui (kdelibs): Attempt to use QAction "" with KXMLGUIFactory! 
okular(22218)/kdeui (kdelibs) KXMLGUIClient::~KXMLGUIClient: 0x15637528 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.

Reproducible: Always

Steps to Reproduce:
open some pdf



OS: Linux (x86_64) release 2.6.34.7-0.5-desktop
Compiler: gcc
Comment 1 Kevin Range 2011-01-16 20:28:43 UTC
I am seeing this with okular 0.11.80 with KDE 4.5.85 as well.  Disabling all debug output in KDebugDialog does not silence this annoying jabber that clogs my terminals.
Comment 2 Albert Astals Cid 2011-01-23 03:05:03 UTC
Adding David as it is a warning he introduced recently.

David, i can "fix" this by doing

Index: shell/shell.cpp
===================================================================
--- shell/shell.cpp     (revision 1214938)
+++ shell/shell.cpp     (working copy)
@@ -42,6 +42,7 @@
 #include <ktoggleaction.h>
 #include <ktogglefullscreenaction.h>
 #include <kactioncollection.h>
+#include <kxmlguifactory.h>
 
 // local includes
 #include "kdocumentviewer.h"
@@ -118,8 +119,12 @@
 
 Shell::~Shell()
 {
-    if ( m_part ) writeSettings();
-    delete m_part;
+    if ( m_part )
+    {
+        writeSettings();
+        factory()->removeClient( m_part );
+        delete m_part;
+    }
     if ( m_args )
         m_args->clear();
 }

But i feel it is somehow weird i have to call 
   factory()->removeClient( m_part );
when i never called addClient().

Is the warning too aggressive? Or do we really ahve to call removeClient before deleting our parts?
Comment 3 Frank Niethardt 2011-01-31 18:23:05 UTC
okular(7862)/kdeui (kdelibs): Attempt to use QAction "" with KXMLGUIFactory! 
okular(7862)/kdeui (kdelibs) KXMLGUIClient::~KXMLGUIClient: 0xfacc28 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes. 

$ okular --version
Qt: 4.7.1
KDE: 4.6.00 (4.6.0)
Okular: 0.12

Archlinux
Comment 4 michael 2011-04-04 02:13:51 UTC
This should be handled by whatever code is doing the adding not okular. It fact this should be done at the time the warning is given even if a warning message must still be displayed. This is how android handles issues of this nature. The system makes the correction for the issue for the sake of stability but still complains because the real fix needs to be done elsewhere.
Comment 5 michael 2011-04-04 02:26:01 UTC
Ignore the last part of that comment.
Comment 6 michael 2011-04-04 04:11:27 UTC
Excerpt from kxmlguiclient.cpp at the source of the warning:
  if ( d->m_factory ) {
      kWarning(240) << this << "deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.";
    d->m_factory->forgetClient(this);
  }

Considering that the removal is autpmaticly done the warning is a bit too strong. There won't be any stability or leak issues as long as the library performs the removal.
Comment 7 Martin von Gagern 2011-04-11 13:44:45 UTC
(In reply to comment #4)
> This should be handled by whatever code is doing the adding not okular.

In case this helps, here is a backtrace from my Gentoo system of where the adding was done for the object whose address is named in the error message:

#0  KXMLGUIClient::setFactory (this=0x6e9f68, factory=0x80e610)
    at kdelibs-4.6.2/kdeui/xmlgui/kxmlguiclient.cpp:599
#1  0x00007ffff73a38cf in KXMLGUIFactory::addClient (this=0x80e610, 
    client=0x6e9f68)
    at kdelibs-4.6.2/kdeui/xmlgui/kxmlguifactory.cpp:274
#2  0x00007ffff7bb7804 in KParts::MainWindow::createGUI (this=0x702e50, part=
    0x6e9cf0)
    at kdelibs-4.6.2/kparts/mainwindow.cpp:126
#3  0x000000000040a5c4 in Shell::init (this=0x702e50)
    at okular-4.6.2/okular/shell/shell.cpp:96
#4  0x000000000040a91e in Shell::Shell (...)
    at okular-4.6.2/okular/shell/shell.cpp:61
#5  0x0000000000408bd0 in main (...)
    at okular-4.6.2/okular/shell/main.cpp:72

I couldn't say which of these should be responsible for the cleanup, though.
Comment 8 michael 2011-04-12 04:27:29 UTC
See https://bugs.kde.org/show_bug.cgi?id=266538 as well. This bug is similar to that one a might be a duplicate.
Comment 9 David Faure 2011-04-15 12:22:54 UTC
I wrote "this will leak" for a good reason. It does leak, forgetClient() can only do some basic cleanup (forgetting this client, like the name says) but not delete associated containers, that caused crashes (e.g. when they were being deleted already).
Comment 10 michael 2011-04-16 22:40:09 UTC
Since this apparently cann't be handled by the library the patch mettioned here would seem to be the only opition.
Comment 11 Christoph Feck 2011-04-21 14:53:09 UTC
*** Bug 271052 has been marked as a duplicate of this bug. ***
Comment 12 Davor Cubranic 2011-06-16 18:49:22 UTC
Why is this warning printed out even if though I turn off debugging output in kdebugdialog? Can it be turned off by editing my kdebugrc?
Comment 13 bkorb 2011-06-16 20:40:06 UTC
The fix has been known for 5 months now.  Please fix it.
Polish the fix later, if necessary.
Comment 14 Albert Astals Cid 2011-06-16 21:06:33 UTC
David, can you confirm that i should be calling factory()->removeClient( m_part ); even if i never called addClient()?
Comment 15 michael 2011-08-18 21:16:32 UTC
The logical place to do this would seem to me to be in kparts/mainwindow.cpp. I am testing a patch to KParts::~MainWindow. Hopefully this doesn't cause problems. Would still like to here back from David on this.
Comment 16 David 2011-10-09 02:40:50 UTC
you may know this, but opening a file in kate from the command line does this also, kubuntu 11.04
Comment 17 michael 2011-10-09 03:35:50 UTC
The kate issue is fixed now in the KDE ppa. Don't know if its been pushed out on any other stable releases yet. kate had mutiple bugs with its own code where it called addClient but did not call removeClient. What I was trying didn't stop the message for Okular. I didn't look to much at why.
Comment 18 Orion Poplawski 2011-10-13 20:00:26 UTC
*** Bug 283942 has been marked as a duplicate of this bug. ***
Comment 19 Albert Astals Cid 2011-12-11 17:11:19 UTC
*** Bug 288650 has been marked as a duplicate of this bug. ***
Comment 20 Jonathan Ryshpan 2011-12-23 19:20:06 UTC
The bug continues in the latest release from Fedora-16, namely 
okular-4.7.4-1.fc16.x86_64.  A message like:

okular(3746)/kdeui (kdelibs) KXMLGUIClient::~KXMLGUIClient: 0x293f978 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.

Shows up on the console whenever okular is run from the console.
Comment 21 Pino Toscano 2012-03-05 10:45:48 UTC
*** Bug 295358 has been marked as a duplicate of this bug. ***
Comment 22 marfcg 2012-03-16 21:37:29 UTC
I'm using Ubuntu 11.10 with okular version 4:4.7.4-0ubuntu0.1 and also have the same issue.
In fact, everytime I open a file (png, pdf, ...) in okular I get this messages:
okular(16596)/kdecore (KConfigSkeleton) KCoreConfigSkeleton::writeConfig:
okular(16596)/kdecore (KConfigSkeleton) KCoreConfigSkeleton::writeConfig:
okular(16596)/kdecore (KConfigSkeleton) KCoreConfigSkeleton::writeConfig:

And when I close the window I get the famous
okular(16596)/kdeui (kdelibs) KXMLGUIClient::~KXMLGUIClient: 0x15a4938 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.

Ubuntu 11.10
Linux 3.0.0-16-generic x86_64
Comment 23 Jekyll Wu 2012-03-22 06:49:40 UTC
*** Bug 295005 has been marked as a duplicate of this bug. ***
Comment 24 Edub Kendo 2012-04-05 07:40:58 UTC
I'm also seeing this exact issue on Ubuntu 11.10. Would like to know how to fix it. For now I'm not running okular from the terminal, but this is an annoyance. Also, I'm not sure if running it from the GUI actually resolves the problem, or if it just doesnt report it the way the terminal does.
Comment 25 Graeme Hewson 2012-06-11 08:02:02 UTC
In KDE 4.8.3 I'm only seeing the "deleted without having been removed from the factory first" message, and only when I quit Okular.
Comment 26 Henry Pfeil 2012-08-07 19:27:01 UTC
Fedora 17, AMD64
These error messages appear on the console that launched XFCE4 (startx).
Okular, Ark. Common tags: "/kdeui (kdelibs)" 
I stumbled about among the kdeui sources and noticed the warning comes from the destructor KXMLGUIClient::~KXMLGUIClient() which deletes the m_parents. Perhaps there is a possible race condition with the factory destructor KXMLGUIFactory::~KXMLGUIFactory() which deletes the m_client instances? Pardon my feeble grasp of the code, just looking for possible ways the Client and Factory destructors could interact.
Comment 27 David Faure 2012-08-08 16:32:16 UTC
Qt GUI code runs in a single thread, there can't be race conditions.

This is about removeClient() not being called, after addClient(). However I don't understand comment #14, if you never call addClient(), then I'm missing a piece of the puzzle (who adds the client?).
Comment 28 Albert Astals Cid 2012-08-08 22:40:55 UTC
(In reply to comment #27)
> if you never call addClient(), then I'm
> missing a piece of the puzzle (who adds the client?).

I have no clue, something in kdelibs?
Comment 29 michael 2012-08-08 22:44:38 UTC
(In reply to comment #28)
> (In reply to comment #27)
> > if you never call addClient(), then I'm
> > missing a piece of the puzzle (who adds the client?).
> 
> I have no clue, something in kdelibs?

That would seem to be indicated by comment 7. There's a stack trace there that should shed some light.
Comment 30 Albert Astals Cid 2012-08-09 22:58:39 UTC
Moving to kdelibs, as Michael says, it is obvious in comment 7 that the addClient call is done by KParts::MainWindow::createGUI, so it is KParts::MainWindow that should care about calling removeClient too
Comment 31 Albert Astals Cid 2012-08-12 11:54:38 UTC
Git commit 4eee1f34793076d3f7ab4dccff87724806da6141 by Albert Astals Cid.
Committed on 12/08/2012 at 13:50.
Pushed by aacid into branch 'KDE/4.9'.

Do not delete the Part on the Shell destructor

The machinery in KParts/QObject is already doing it and this way we don't get the
KXMLGUIClient::~KXMLGUIClient: 0x15637528 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.
warning

I'm not sure if calling this a kdelibs bug yet or not though :D
FIXED-IN: 4.9.1

M  +1    -1    shell/shell.cpp

http://commits.kde.org/okular/4eee1f34793076d3f7ab4dccff87724806da6141
Comment 32 Albert Astals Cid 2012-08-12 11:58:16 UTC
Git commit 035c32996ea45aa682db1d18460606ef12c5e6b3 by Albert Astals Cid.
Committed on 12/08/2012 at 13:50.
Pushed by aacid into branch 'master'.

Do not delete the Part on the Shell destructor

The machinery in KParts/QObject is already doing it and this way we don't get the
KXMLGUIClient::~KXMLGUIClient: 0x15637528 deleted without having been removed from the factory first. This will leak standalone popupmenus and could lead to crashes.
warning

I'm not sure if calling this a kdelibs bug yet or not though :D
FIXED-IN: 4.9.1

M  +1    -1    shell/shell.cpp

http://commits.kde.org/okular/035c32996ea45aa682db1d18460606ef12c5e6b3
Comment 33 Toralf Förster 2012-08-12 16:10:40 UTC
patch works for 4.8.5. too
Comment 34 Albert Astals Cid 2012-08-12 18:12:08 UTC
4.8.x is dead development wise, distributions are free to do as they please under their responsability :-)
Comment 35 Markus Trippelsdorf 2012-08-18 16:26:59 UTC
This patch causes Okular to spin in the background (consuming 100% CPU) after I've closed it
and the window vanished.
This happens when I open a pdf and close it (almost) immediately.

"perf top" shows:
    47.83%  libQtCore.so.4.8.2       [.] QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) 
    15.58%  libQtCore.so.4.8.2       [.] QMutex::unlock()                                        
    14.04%  libokularcore.so.1.9.0 [.] Okular::Document::closeDocument()                       
    13.35%  libQtCore.so.4.8.2       [.] QMutex::lock(
Comment 36 Albert Astals Cid 2012-08-18 16:38:55 UTC
Good catch! Can you please update your okular git checkout and make sure my commit fixes the problem you are experimenting?
Comment 37 Markus Trippelsdorf 2012-08-18 16:47:50 UTC
(In reply to comment #36)
> Good catch! Can you please update your okular git checkout and make sure my
> commit fixes the problem you are experimenting?

Yes everything works as expected again.
Thanks for the quick fix.
Comment 38 Toralf Förster 2012-08-18 18:23:47 UTC
(In reply to comment #35)
> This happens when I open a pdf and close it (almost) immediately.
Right - same for 4.8.5. If okular is started on the command line, it can be "closed" via ctrl-C.
Because the patch of #comment 32 imade its ways into the Gentoo repository /me wonders where there's a fix for this patch mentioned in #comment 36 ?
Comment 40 Albert Astals Cid 2012-08-24 15:51:03 UTC
Moving it back to okular since at the end the codechange was done there, i'm not enterely sure whose fault it is, but who cares :D
Comment 41 Albert Astals Cid 2012-08-24 15:51:14 UTC
*** Bug 305726 has been marked as a duplicate of this bug. ***