~ubuntu-branches/ubuntu/lucid/amarok/lucid-backports

« back to all changes in this revision

Viewing changes to debian/patches/kubuntu/04_scanning_crash.diff

  • Committer: Bazaar Package Importer
  • Author(s): Scott Kitterman
  • Date: 2011-03-03 10:27:39 UTC
  • mfrom: (114.1.20 natty)
  • Revision ID: james.westby@ubuntu.com-20110303102739-ar67wpa6mllo59n2
Tags: 2:2.4.0-0ubuntu4~lucid1
* Source backport to lucid (LP: #728447)
  - Drop version requirement on libindicate-qt-dev build-dep

Show diffs side-by-side

added added

removed removed

Lines of Context:
1
 
Fixes potential crashes during scanning discovered in 2.2.2 after tagging.
2
 
Jeff Mitchell <mitchell@kde.org>
3
 
 
4
 
 
5
 
diff --git a/src/collection/sqlcollection/ScanResultProcessor.cpp b/src/collection/sqlcollection/ScanResultProcessor.cpp
6
 
index b680623..68b31df 100644
7
 
--- a/src/collection/sqlcollection/ScanResultProcessor.cpp
8
 
+++ b/src/collection/sqlcollection/ScanResultProcessor.cpp
9
 
@@ -42,19 +42,59 @@ ScanResultProcessor::ScanResultProcessor( SqlCollection *collection )
10
 
 ScanResultProcessor::~ScanResultProcessor()
11
 
 {
12
 
     //everything has a URL, so enough to just delete from here
13
 
+    QSet<QStringList*> currSet; //prevent double deletes
14
 
     foreach( QStringList *list, m_urlsHashByUid )
15
 
-        delete list;
16
 
+    {
17
 
+        if( list )
18
 
+        {
19
 
+            if( !currSet.contains( list ) )
20
 
+            {
21
 
+                delete list;
22
 
+                currSet.insert( list );
23
 
+            }
24
 
+        }
25
 
+        else
26
 
+            debug() << "GAAH! Tried to double-delete a value in m_urlsHashByUid";
27
 
+    }
28
 
     foreach( QLinkedList<QStringList*> *list, m_albumsHashByName )
29
 
     {
30
 
-        foreach( QStringList *slist, *list )
31
 
-            delete slist;
32
 
-        delete list;
33
 
+        if( list )
34
 
+        {
35
 
+            foreach( QStringList *slist, *list )
36
 
+            {
37
 
+                if( slist )
38
 
+                {
39
 
+                    if( !currSet.contains( slist ) )
40
 
+                    {
41
 
+                        delete slist;
42
 
+                        currSet.insert( slist );
43
 
+                    }
44
 
+                    else
45
 
+                        debug() << "GAAH! Tried to double-delete a value in m_albumsHashByName";
46
 
+                }
47
 
+            }
48
 
+            delete list;
49
 
+        }
50
 
     }
51
 
     foreach( QLinkedList<QStringList*> *list, m_tracksHashByAlbum )
52
 
     {
53
 
-        foreach( QStringList *slist, *list )
54
 
-            delete slist;
55
 
-        delete list;
56
 
+        if( list )
57
 
+        {
58
 
+            foreach( QStringList *slist, *list )
59
 
+            {
60
 
+                if( slist )
61
 
+                {
62
 
+                    if( !currSet.contains( slist ) )
63
 
+                    {
64
 
+                        delete slist;
65
 
+                        currSet.insert( slist );
66
 
+                    }
67
 
+                    else
68
 
+                        debug() << "GAAH! Tried to double-delete a value in m_tracksHashByAlbum";
69
 
+                }
70
 
+            }
71
 
+            delete list;
72
 
+        }
73
 
     }
74
 
 }
75
 
 
76
 
@@ -67,11 +107,11 @@ ScanResultProcessor::setScanType( ScanType type )
77
 
 void
78
 
 ScanResultProcessor::addDirectory( const QString &dir, uint mtime )
79
 
 {
80
 
-    DEBUG_BLOCK
81
 
-    debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
82
 
+    //DEBUG_BLOCK
83
 
+    //debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
84
 
     if( dir.isEmpty() )
85
 
     {
86
 
-        debug() << "got directory with no path from the scanner, not adding";
87
 
+        //debug() << "got directory with no path from the scanner, not adding";
88
 
         return;
89
 
     }
90
 
     setupDatabase();
91
 
@@ -254,7 +294,7 @@ ScanResultProcessor::rollback()
92
 
 void
93
 
 ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
94
 
 {
95
 
-//     DEBUG_BLOCK
96
 
+    //DEBUG_BLOCK
97
 
     setupDatabase();
98
 
     //using the following heuristics:
99
 
     //if more than one album is in the dir, use the artist of each track as albumartist
100
 
@@ -274,24 +314,54 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
101
 
         if( row.value( Field::ALBUM ).toString() != album )
102
 
             multipleAlbums = true;
103
 
     }
104
 
+
105
 
     if( multipleAlbums || album.isEmpty() || artists.size() == 1 )
106
 
     {
107
 
         foreach( const QVariantMap &row, data )
108
 
         {
109
 
-            int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
110
 
-            addTrack( row, artist );
111
 
+            QString uid = row.value( Field::UNIQUEID ).toString();
112
 
+            if( m_uidsSeenThisScan.contains( uid ) )
113
 
+            {
114
 
+                QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
115
 
+                                             m_urlsHashByUid[uid] != 0 ) ?
116
 
+                                             MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
117
 
+                debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
118
 
+                           "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
119
 
+            }
120
 
+            else
121
 
+            {
122
 
+                int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
123
 
+                //debug() << "artist found = " << artist;
124
 
+                addTrack( row, artist );
125
 
+                m_uidsSeenThisScan.insert( uid );
126
 
+            }
127
 
         }
128
 
     }
129
 
     else
130
 
     {
131
 
         QString albumArtist = findAlbumArtist( artists, data.count() );
132
 
+        //debug() << "albumArtist found = " << albumArtist;
133
 
         //an empty string means that no albumartist was found
134
 
         int artist = albumArtist.isEmpty() ? 0 : genericId( &m_artists, albumArtist, &m_nextArtistNum );
135
 
+        //debug() << "artist found = " << artist;
136
 
 
137
 
         //debug() << "albumartist " << albumArtist << "for artists" << artists;
138
 
         foreach( const QVariantMap &row, data )
139
 
         {
140
 
-            addTrack( row, artist );
141
 
+            QString uid = row.value( Field::UNIQUEID ).toString();
142
 
+            if( m_uidsSeenThisScan.contains( uid ) )
143
 
+            {
144
 
+                QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
145
 
+                                             m_urlsHashByUid[uid] != 0 ) ?
146
 
+                                             MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
147
 
+                debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
148
 
+                           "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
149
 
+            }
150
 
+            else
151
 
+            {
152
 
+                addTrack( row, artist );
153
 
+                m_uidsSeenThisScan.insert( uid );
154
 
+            }
155
 
         }
156
 
     }
157
 
 }
158
 
@@ -299,6 +369,7 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
159
 
 QString
160
 
 ScanResultProcessor::findAlbumArtist( const QSet<QString> &artists, int trackCount ) const
161
 
 {
162
 
+    //DEBUG_BLOCK
163
 
     QMap<QString, int> artistCount;
164
 
     bool featuring;
165
 
     QStringList trackArtists;
166
 
@@ -371,6 +442,7 @@ void
167
 
 ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
168
 
 {
169
 
     //DEBUG_BLOCK
170
 
+    //debug() << "albumArtistId = " << albumArtistId;
171
 
     //amarok 1 stored all tracks of a compilation in different directories.
172
 
     //when using its "Organize Collection" feature
173
 
     //try to detect these cases
174
 
@@ -419,7 +491,15 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
175
 
 
176
 
     //urlId will take care of the urls table part of AFT
177
 
     int url = urlId( path, uid );
178
 
-
179
 
+/*
180
 
+    foreach( QString key, m_urlsHashByUid.keys() )
181
 
+    debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
182
 
+    foreach( int key, m_urlsHashById.keys() )
183
 
+    debug() << "Key: " << key << ", list: " << *m_urlsHashById[key];
184
 
+    typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
185
 
+    foreach( blahType key, m_urlsHashByLocation.keys() )
186
 
+    debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
187
 
+*/
188
 
     QStringList *trackList = new QStringList();
189
 
     int id = m_nextTrackNum;
190
 
     //debug() << "Appending new track number with tracknum: " << id;
191
 
@@ -470,7 +550,7 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
192
 
     //insert into hashes
193
 
     if( m_tracksHashByUrl.contains( url ) && m_tracksHashByUrl[url] != 0 )
194
 
     {
195
 
-        //debug() << "m_tracksHashByUrl contains the url!";
196
 
+        //debug() << "m_tracksHashByUrl already contains url " << url;
197
 
         //need to replace, not overwrite/add a new one
198
 
         QStringList *oldValues = m_tracksHashByUrl[url];
199
 
         QString oldId = oldValues->at( 0 );
200
 
@@ -490,8 +570,24 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
201
 
         m_tracksHashById.insert( id, trackList );
202
 
     }
203
 
 
204
 
+    //debug() << "album = " << album;
205
 
+
206
 
     if( m_tracksHashByAlbum.contains( album ) && m_tracksHashByAlbum[album] != 0 )
207
 
-        m_tracksHashByAlbum[album]->append( trackList );
208
 
+    {
209
 
+        //contains isn't the fastest on linked lists, but in reality this is on the order of maybe
210
 
+        //ten quick pointer comparisons per track on average...probably lower
211
 
+        //debug() << "trackList is " << trackList;
212
 
+        if( !m_tracksHashByAlbum[album]->contains( trackList ) )
213
 
+        {
214
 
+            //debug() << "appending trackList to m_tracksHashByAlbum";
215
 
+            m_tracksHashByAlbum[album]->append( trackList );
216
 
+        }
217
 
+        else
218
 
+        {
219
 
+            //debug() << "not appending trackList to m_tracksHashByAlbum";
220
 
+        }
221
 
+
222
 
+    }
223
 
     else
224
 
     {
225
 
         QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
226
 
@@ -595,6 +691,8 @@ ScanResultProcessor::albumId( const QString &album, int albumArtistId )
227
 
         QLinkedList<QStringList*> *list = m_albumsHashByName[album];
228
 
         foreach( QStringList *slist, *list )
229
 
         {
230
 
+            //debug() << "albumArtistId = " << albumArtistId;
231
 
+            //debug() << "Checking list: " << *slist;
232
 
             if( slist->at( 2 ).isEmpty() && albumArtistId == 0 )
233
 
             {
234
 
                 //debug() << "artist is empty and albumArtistId = 0, returning " << slist->at( 0 );
235
 
@@ -631,7 +729,10 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
236
 
     albumList->append( QString() );
237
 
     m_albumsHashById[returnedNum] = albumList;
238
 
     if( m_albumsHashByName.contains( album ) && m_albumsHashByName[album] != 0 )
239
 
-        m_albumsHashByName[album]->append( albumList );
240
 
+    {
241
 
+        if( !m_albumsHashByName[album]->contains( albumList ) )
242
 
+            m_albumsHashByName[album]->append( albumList );
243
 
+    }
244
 
     else
245
 
     {
246
 
         QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
247
 
@@ -645,7 +746,7 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
248
 
 int
249
 
 ScanResultProcessor::urlId( const QString &url, const QString &uid )
250
 
 {
251
 
-    /*
252
 
+/*
253
 
     DEBUG_BLOCK
254
 
     foreach( QString key, m_urlsHashByUid.keys() )
255
 
     debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
256
 
@@ -654,8 +755,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
257
 
     typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
258
 
     foreach( blahType key, m_urlsHashByLocation.keys() )
259
 
     debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
260
 
-    */
261
 
262
 
+*/
263
 
     QFileInfo fileInfo( url );
264
 
     const QString dir = fileInfo.absoluteDir().absolutePath();
265
 
     int dirId = directoryId( dir );
266
 
@@ -665,6 +765,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
267
 
     QPair<int, QString> locationPair( deviceId, rpath );
268
 
     //debug() << "in urlId with url = " << url << " and uid = " << uid;
269
 
     //debug() << "checking locationPair " << locationPair;
270
 
+/*
271
 
     if( m_urlsHashByLocation.contains( locationPair ) )
272
 
     {
273
 
         QStringList values;
274
 
@@ -674,6 +775,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
275
 
             values << "zero";
276
 
         //debug() << "m_urlsHashByLocation contains it! It is " << values;
277
 
     }
278
 
+*/
279
 
     QStringList currUrlIdValues;
280
 
     if( m_urlsHashByUid.contains( uid ) && m_urlsHashByUid[uid] != 0 )
281
 
         currUrlIdValues = *m_urlsHashByUid[uid];
282
 
@@ -717,6 +819,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
283
 
             //debug() << "m_urlsHashByUid contains this UID, updating deviceId and path";
284
 
             QStringList *list = m_urlsHashByUid[uid];
285
 
             //debug() << "list from UID hash is " << list << " with values " << *list;
286
 
+            QPair<int, QString> oldLocationPair( list->at( 1 ).toInt(), list->at( 2 ) );
287
 
             list->replace( 1, QString::number( deviceId ) );
288
 
             list->replace( 2, rpath );
289
 
             list->replace( 3, QString::number( dirId ) );
290
 
@@ -737,6 +840,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
291
 
                 delete oldList;
292
 
             }
293
 
             m_urlsHashByLocation[locationPair] = list;
294
 
+            m_urlsHashByLocation.remove( oldLocationPair );
295
 
         }
296
 
         m_permanentTablesUrlUpdates.insert( uid, url );
297
 
         m_changedUrls.insert( uid, QPair<QString, QString>( MountPointManager::instance()->getAbsolutePath( currUrlIdValues[1].toInt(), currUrlIdValues[2] ), url ) );
298
 
@@ -751,6 +855,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
299
 
         {
300
 
             QStringList *list = m_urlsHashByLocation[locationPair];
301
 
             //debug() << "Replacing hash " << list->at( 4 ) << " with " << uid;
302
 
+            QString oldId = list->at( 4 );
303
 
             list->replace( 4, uid );
304
 
             if( m_urlsHashByUid.contains( uid )
305
 
                 && m_urlsHashByUid[uid] != 0 
306
 
@@ -762,6 +867,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
307
 
                 delete oldList;
308
 
             }
309
 
             m_urlsHashByUid[uid] = list;
310
 
+            m_urlsHashByUid.remove( oldId );
311
 
         }
312
 
         m_permanentTablesUidUpdates.insert( url, uid );
313
 
         m_changedUids.insert( currUrlIdValues[4], uid );
314
 
@@ -855,7 +961,8 @@ ScanResultProcessor::directoryId( const QString &dir )
315
 
 int
316
 
 ScanResultProcessor::checkExistingAlbums( const QString &album )
317
 
 {
318
 
-//     DEBUG_BLOCK
319
 
+    //DEBUG_BLOCK
320
 
+    //debug() << "looking for album " << album;
321
 
     // "Unknown" albums shouldn't be handled as compilations
322
 
     if( album.isEmpty() )
323
 
         return 0;
324
 
@@ -865,7 +972,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
325
 
     //it's probably a compilation.
326
 
     //this handles A1 compilations that were automatically organized by Amarok
327
 
     if( !m_albumsHashByName.contains( album ) || m_albumsHashByName[album] == 0 )
328
 
+    {
329
 
+        //debug() << "hashByName doesn't contain album, or it's zero";
330
 
         return 0;
331
 
+    }
332
 
 
333
 
     QStringList trackIds;
334
 
     QLinkedList<QStringList*> *llist = m_albumsHashByName[album];
335
 
@@ -915,8 +1025,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
336
 
         }
337
 
     }
338
 
 
339
 
+    //debug() << "trackIds = " << trackIds;
340
 
     if( trackIds.isEmpty() )
341
 
     {
342
 
+        //debug() << "trackIds empty, returning zero";
343
 
         return 0;
344
 
     }
345
 
     else
346
 
@@ -933,6 +1045,7 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
347
 
                 list->replace( 3, compilationString );
348
 
             }
349
 
         }
350
 
+        //debug() << "returning " << compilationId;
351
 
         return compilationId;
352
 
     }
353
 
 }
354
 
@@ -1167,6 +1280,17 @@ ScanResultProcessor::copyHashesToTempTables()
355
 
     foreach( blahType key, m_urlsHashByLocation.keys() )
356
 
         debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
357
 
     debug() << "Next album num: " << m_nextAlbumNum;
358
 
+
359
 
+    foreach( int key, m_tracksHashById.keys() )
360
 
+        debug() << "Key: " << key << ", list: " << *m_tracksHashById[key];
361
 
+    foreach( int key, m_tracksHashByUrl.keys() )
362
 
+        debug() << "Key: " << key << ", list: " << *m_tracksHashByUrl[key];
363
 
+    foreach( int key, m_tracksHashByAlbum.keys() )
364
 
+    {
365
 
+        debug() << "Key: " << key;
366
 
+        foreach( QStringList* item, *m_tracksHashByAlbum[key] )
367
 
+            debug() << "list: " << item << " is " << *item;
368
 
+    }
369
 
     */
370
 
  
371
 
     DEBUG_BLOCK
372
 
diff --git a/src/collection/sqlcollection/ScanResultProcessor.h b/src/collection/sqlcollection/ScanResultProcessor.h
373
 
index 71afc98..c7f200f 100644
374
 
--- a/src/collection/sqlcollection/ScanResultProcessor.h
375
 
+++ b/src/collection/sqlcollection/ScanResultProcessor.h
376
 
@@ -94,6 +94,7 @@ class ScanResultProcessor : public QObject
377
 
         QMap<QString, int> m_directories;
378
 
         QMap<QString, QList< QPair< QString, QString > > > m_imageMap;
379
 
 
380
 
+        QSet<QString> m_uidsSeenThisScan;
381
 
         QHash<QString, uint> m_filesInDirs;
382
 
 
383
 
         TrackUrls m_changedUids; //not really track urls