search

mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.51k stars 1.28k forks source link

Modified track info is lost on a crash #7839

Open mixxxbot opened 2 years ago

mixxxbot commented 2 years ago

Reported by: ferranpujolcamins Date: 2015-01-31T09:43:57Z Status: Confirmed Importance: Wishlist Launchpad Issue: lp1416636

How to reproduce: -Open Mixxx from command line -Modify a track title for example -Send Mixxx SIGINT -Open Mixxx again: track changes are lost

Mixxx should save tracks to database as soon as they are changed or in a reasonable short time after.

mixxxbot commented 2 years ago

Commented by: daschuer Date: 2015-01-31T11:31:10Z

This probably not that trivial. We want to be reliable as much as possible, but reduce the DB HD access to a minimum since it is driven from the GU thread and may block user input for a moment. Maybe we have to tweak the current situation. This will be much easier once we have merge the dedicated DB thread pull request of Nazar into master. https://github.com/mixxxdj/mixxx/pull/73

mixxxbot commented 2 years ago

Commented by: ferranpujolcamins Date: 2015-07-20T14:55:57Z

I would like to fix this bug. What is the state of the library concurrency refactoring? Once that is solved, is it ok to just push the changes to the database as soon as they happen?

mixxxbot commented 2 years ago

Commented by: daschuer Date: 2015-07-20T15:26:52Z

you can follow the progress here: https://github.com/mixxxdj/mixxx/pull/629 MK-42 has started to privatize the Library access, in a way that future code is forced to use the new API. I am currently working to merge master step by step into the branch.
We are still fighting various concurrency issues. Any comment or help is welcome.

You may fix this bug by directly call the database assuming it will be done asynchrone in a Database thread. We have to delay the merge until the database theads is available. Or you may target the solution to MK-42 branch. This way you may get familial with it and you may find issues on the way.

mixxxbot commented 2 years ago

Commented by: ferranpujolcamins Date: 2015-07-20T18:28:31Z

I've forked MK-42's branch and track info modifications are not stored on a normal mixxx restart. I'll just make a PR from master.

mixxxbot commented 2 years ago

Commented by: ferranpujolcamins Date: 2015-07-22T19:54:22Z

I'd need a hand here. I've seen that TrackDAO::finish() is what commits changes to the database, is this right? I'm not familiar with this part of the code so I don't really know what parts of this functions should be called regularly in order to solve the bug.

I guess I should add a new method to TrackDAO that commits the pending changes to the database. Would this lead to concurrency problems? I'm have no experience in concurrent code so I don't have a clue here. Would it be reasonable to call this each time a change to the database is made?

mixxxbot commented 2 years ago

Commented by: daschuer Date: 2015-07-23T07:20:43Z

The track is saved just before the destructor of the TrackInfo Object is called. https://github.com/mixxxdj/mixxx/blob/e94a1a00e69a572166822e5a6990fc1edae98302/src/library/dao/trackdao.cpp#L53

Weak cache that controls that we have only one instance of a single track in Mixxx. If a track is not referenced, it is saved. In addition we have a cache that a strong reference of 5 recently used tracks This means we have always at least 5 unsaved and probably dirty tracks around. This number can be increase it we have more referenced tracks somewhere else in Mixxx.

During the solution of the bug, you may watch the size of the weak cache. We need to be sure, that it is at a reasonable size at any moment. If it is to high, we have a pending reference of an unused track, that prevents the track to be saved.

We may also discuss the sense of the strong cache. What is the benefit of it. If we just remove it any track is saved once it is not used. Is there a reason in the normal Mixxx workflow, that a track is used unused and used again short after?