Review Request 118988: AsyncMusicBrainzLookup: fix a crash and a memory leak

[Frank Schaefer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118988/
-----------------------------------------------------------

Review request for KDE Multimedia.


Bugs: 336678
    http://bugs.kde.org/show_bug.cgi?id=336678


Repository: libkcddb


Description
-------

>From the documentation of QThread::~QThread():
"... Note that deleting a QThread object will not stop the execution of the thread it manages.
Deleting a running QThread (i.e. isFinished() returns false) will probably result in a program crash.
Wait for the finished() signal before deleting the QThread."

The destructor of class AsyncMusicBrainzLookup currently just deletes the lookup thread object
and does _not_ wait for the running thread to finish.
This causes crashes with the Audex CDDA extraction tool and likely also other users of libkcddb.

In addition to that, we are leaking the memory of a previously intstanciated lookup thread object if
the lookup() method is called multiple times.

Solve both issues by making the lookup thread object "self-destructive".
This way we can maintain the current behavior of the destructor (non-blocking) and
the lookup() method (always start a new lookup without canceling pending lookups).
As part of the solution the lookup thread is modifed to deliver the lookup results
directly with the "lookupFinished" signal. This has two benefits:
1.) the queued signal-slot connection avoids thread synchronization/locking issues
2.) some class members become obsolete


Diffs
-----

  libkcddb/musicbrainz/asyncmusicbrainzlookup.h 373fb16 
  libkcddb/musicbrainz/asyncmusicbrainzlookup.cpp c13c94f 

Diff: https://git.reviewboard.kde.org/r/118988/diff/


Testing
-------

yes


Thanks,

Frank Schaefer

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20140628/0b2c326a/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia