D24150: Fix crash on exit in kio_file

[David Faure]

dfaure closed this revision.

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Ah, I see, OK.

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Loïc Yhuel]

hwti added a comment.


  In D24150#536169 , @dfaure wrote:
  
  > I (thought I) fixed this crash already in commit 512967f6f4e887d4a5a0 
 by 
removing the ::exit() call (which appears in the backtrace of that bug report).
  >
  > This being said, I'm not objecting to this patch, but I am wondering if you 
can still experience this crash with a kio that includes my commit above (from 
Sep 6).
  
  
  QCoreGlobalData destructor is called on any exit, even when returning from 
main (but there is an exit call in kdeinit5 anyway).
  If there is no other possible exit() call before the kdemain function in 
file.cpp returns, then you avoided the issue for Qt >= 5.12 : destroying the 
QTextCodec unregisters it, so it isn't in the list any more when 
QCoreGlobalData destructor is called.
  But older Qt versions do not have this logic, so QCoreGlobalData destructor 
often crashes when trying to call the LegacyTextCodec destructor, since the 
object has already been destroyed and is above the stack pointer. Even if it 
doesn't crash, free() would then be called on a stack address.

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[David Faure]

dfaure added a comment.


  I (thought I) fixed this crash already in commit 512967f6f4e887d4a5a0 
 by 
removing the ::exit() call (which appears in the backtrace of that bug report).
  
  This being said, I'm not objecting to this patch, but I am wondering if you 
can still experience this crash with a kio that includes my commit above (from 
Sep 6).

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Loïc Yhuel]

hwti updated this revision to Diff 66633.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24150?vs=66630=66633

REVISION DETAIL
  https://phabricator.kde.org/D24150

AFFECTED FILES
  src/ioslaves/file/file.cpp

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Loïc Yhuel]

hwti added a comment.


  In D24150#536113 , @aacid wrote:
  
  > why the void? do you get a warning otherwise?
  
  
  No, but it is used in several places in the repository (and in Qt), so I 
suppose it might generate one depending on the compiler.
  
  > Also a comment in that line quoting the Qt docs "Note that you should not 
delete codecs yourself: once created they become Qt's responsibility." makes 
sense IMHO otherwise someone will move it back to the old code to not have a 
leak
  
  OK, done (but the void should already show that not storing it in a variable, 
so not deleting it, is deliberate).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Albert Astals Cid]

aacid added a comment.


  why the void? do you get a warning otherwise?
  
  Also a comment in that line quoting the Qt docs "Note that you should not 
delete codecs yourself: once created they become Qt's responsibility." makes 
sense IMHO otherwise someone will move it back to the old code to not have a 
leak

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: aacid, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Ahmad Samir]

ahmadsamir added reviewers: dfaure, broulik.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24150

To: hwti, cfeck, dfaure, broulik
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24150: Fix crash on exit in kio_file

[Loïc Yhuel]

hwti created this revision.
hwti added a reviewer: cfeck.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
hwti requested review of this revision.

REVISION SUMMARY
  All QTextCodec are deleted by QCoreGlobalData on exit, so they must be 
allocated on the heap.
  Before Qt 5.12, it is even not allowed to delete them.
  
  BUG: 408797

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24150

AFFECTED FILES
  src/ioslaves/file/file.cpp

To: hwti, cfeck
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns