Bug 323156

Summary:	crashes if a file in the sqlcollection is replaced while Amarok is running
Product:	[Applications] amarok	Reporter:	Konrad Zemek <konrad.zemek>
Component:	Collections/Local	Assignee:	Amarok Developers <amarok-bugs-dist>
Status:	RESOLVED FIXED
Severity:	crash	CC:	matej, myriam, ralf-engels
Priority:	NOR
Version:	2.7-git
Target Milestone:	2.8
Platform:	Arch Linux
OS:	Linux
Latest Commit:	http://commits.kde.org/amarok/b0121dbb0d7f72f7f78998e1c075d73d177ba8f1	Version Fixed In:	2.9
Sentry Crash Report:

Description Konrad Zemek 2013-08-04 12:42:02 UTC

Amarok crashes on asserts if an already indexed music file is replaced while Amarok's running. One of the failed asserts occurs on directory rescan:

amarok: [AbstractScanResultProcessor] got 1 directories 
amarok: BEGIN: virtual void SqlScanResultProcessor::scanSucceeded() 
amarok:   BEGIN: virtual void AbstractScanResultProcessor::scanSucceeded() 
amarok:     [SqlRegistry] SqlRegistry::getDirectory(): update directory "/run/media/konrad/KONDZIU'S/Music/Metallica/Metallica/" (id "160" ) from 1375615468 to 1375615503 UNIX time 
amarok:     [SqlScanResultProcessor] commitAlbum on "Metallica" artist "Metallica"

...

amarok:     [WARNING] [SqlScanResultProcessor] urlsCacheInsert(): found duplicate in path. old Entry(id=1206, path="/run/media/konrad/KONDZIU'S/Music/Metallica/Metallica/12 - The Struggle Within.mp3", dirId=160, uid="amarok-sqltrackuid://f0ba618b68cc70980922fd2fc0f9dcef") will be hidden by the new one in the cache: Entry(id=1607, path="/run/media/konrad/KONDZIU'S/Music/Metallica/Metallica/12 - The Struggle Within.mp3", dirId=160, uid="amarok-sqltrackuid://689cdd9ef0526151ae08a1ebbe3f40c0") 
amarok:     [SqlScanResultProcessor] removeTrack( Entry(id=1206, path="/run/media/konrad/KONDZIU'S/Music/Metallica/Metallica/12 - The Struggle Within.mp3", dirId=160, uid="amarok-sqltrackuid://f0ba618b68cc70980922fd2fc0f9dcef") ) 
ASSERT: "track->urlId() == entry.id" in file /home/konrad/kde/src/amarok/src/core-impl/collections/db/sql/SqlScanResultProcessor.cpp, line 532


the other failed assert occurs after replaced song is played:

amarok: BEGIN: void EngineController::slotFinished() 
amarok:   [EngineController] Track finished completely, updating statistics 
amarok:   [EngineController] slotTrackFinishedPlaying( "[no artist]" - "[no album]" - "1000 Words" , 1 ) 
amarok:   [WARNING] [SqlRegistry] updating uid to an already existing uid. 
amarok:   [ERROR__] [MySqlStorage] "GREPME MySQL-server query failed! (1062) Duplicate entry '2-./run/media/konrad/KONDZIU'S/Music/Final Fantasy/Final Fantasy' for key 'urls_id_rpath' on INSERT INTO urls (deviceid,rpath,directory,uniqueid) VALUES (2,'./run/media/konrad/KONDZIU\'S/Music/Final Fantasy/Final Fantasy X-2 Piano Collec/10 - 1000 Words.mp3',105,'amarok-sqltrackuid://fa4c80d8a5da175c942d5c95f8ee5b3a');" 
amarok:   [WARNING] [SqlRegistryP] Insert failed. 
ASSERT: "track->m_urlId > 0 && "refusing to write non-positive urlId to tracks table, please file a bug"" in file /home/konrad/kde/src/amarok/src/core-impl/collections/db/sql/SqlRegistry_p.cpp, line 270


Reproducible: Sometimes

Steps to Reproduce:
1. Recode: a.mp3 -> a.mp3.new
2. Copy ID3 tags: a.mp3 -> a.mp3.new
3. Remove a.mp3
4. Rename a.mp3.new -> a.mp3
Actual Results:  
Amarok (eventually) fails an assert.

Expected Results:  
Change in track content is picked up, track metadata is updated but otherwise nothing happens.

Amarok is ran from debug build, as otherwise no asserts would evaluate. Effects while running release binary may either be disastrous or nonexistent.

Comment 1 Matěj Laitl 2013-11-06 15:24:29 UTC

Git commit b0121dbb0d7f72f7f78998e1c075d73d177ba8f1 by Matěj Laitl.
Committed on 06/11/2013 at 15:10.
Pushed by laitl into branch 'master'.

SqlRegistry: fix assert fail in SqlScanResultProcessor by always returning the correct track

What happened:
 1. User has 1.mp3, copies it as 1.new.mp3 and edits some tags of
    1.new.mp3. Both files do *not* have AFT tag embedded.
 2. Amarok registers the new file.
 3. Users restarts Amarok (needed to reproduce because of internal state)
 4. User renames 1.new.mp3 back to 1.mp3 by overwriting it, Amarok catches it
 5. Amarok does not associate new 1.mp3 with the old 1.mp3 because it finds
    previous 1.new.mp3 entry by uid (file hash) instead.
 6. Amarok tries to remove entry for the old 1.mp3, but
    SqlRegistry::getTrack( int urlId ) erroneously returns the new entry
 7. SqlScanResultProcessor correctly asserts out, refusing to delete the
    incorrect entry.

The fix is to enhance SqlRegistry::getTrack( int urlId ) to double-check
that it indeed returns the correct entry.
Related: bug 322474
FIXED-IN: 2.9
BACKPORT

M  +2    -0    ChangeLog
M  +24   -10   src/core-impl/collections/db/sql/SqlRegistry.cpp

http://commits.kde.org/amarok/b0121dbb0d7f72f7f78998e1c075d73d177ba8f1

Comment 2 Matěj Laitl 2013-11-06 15:27:44 UTC

> the other failed assert occurs after replaced song is played:
> 
> amarok: BEGIN: void EngineController::slotFinished() 
> amarok:   [EngineController] Track finished completely, updating statistics 
> amarok:   [EngineController] slotTrackFinishedPlaying( "[no artist]" - "[no
> album]" - "1000 Words" , 1 ) 
> amarok:   [WARNING] [SqlRegistry] updating uid to an already existing uid. 
> amarok:   [ERROR__] [MySqlStorage] "GREPME MySQL-server query failed! (1062)
> Duplicate entry '2-./run/media/konrad/KONDZIU'S/Music/Final Fantasy/Final
> Fantasy' for key 'urls_id_rpath' on INSERT INTO urls
> (deviceid,rpath,directory,uniqueid) VALUES
> (2,'./run/media/konrad/KONDZIU\'S/Music/Final Fantasy/Final Fantasy X-2
> Piano Collec/10 - 1000
> Words.mp3',105,'amarok-sqltrackuid://fa4c80d8a5da175c942d5c95f8ee5b3a');" 
> amarok:   [WARNING] [SqlRegistryP] Insert failed. 
> ASSERT: "track->m_urlId > 0 && "refusing to write non-positive urlId to
> tracks table, please file a bug"" in file
> /home/konrad/kde/src/amarok/src/core-impl/collections/db/sql/SqlRegistry_p.
> cpp, line 270

I believe this second assert is just a consequence of the first one, but please test that it indeed isn't reproducible (and also that I really fixed the fist one, too).

Anyways, thanks for a very helpful bug report, Konrad, keep em' coming.

Comment 3 Matěj Laitl 2013-11-06 15:46:49 UTC

*** Bug 323471 has been marked as a duplicate of this bug. ***