Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project crash on loading, controller loading error #4060

Closed
DeRobyJ opened this issue Dec 19, 2017 · 12 comments
Closed

Project crash on loading, controller loading error #4060

DeRobyJ opened this issue Dec 19, 2017 · 12 comments
Assignees
Labels
Milestone

Comments

@DeRobyJ
Copy link
Contributor

DeRobyJ commented Dec 19, 2017

Some projects saved with 1.2 beta versions will always fail to crash.

Affected projects trigger this assertion:

void Song::restoreControllerStates( const QDomElement & element )
{
    QDomNode node = element.firstChild();
    while( !node.isNull() && !isCancelled() )
    {
        Controller * c = Controller::create( node.toElement(), this );
        Q_ASSERT( c != NULL );

        addController( c );

        node = node.nextSibling();
    }
}

backtrace

QtXmlWrapper::loadXMLfile(): empty data
ASSERT: "c != NULL" in file /home/ubuntu/Desktop/lmms/src/core/Song.cpp, line 1292

Program received signal SIGABRT, Aborted.
0x00007ffff3fb6c37 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56    ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.

The bug is OS-independent.
These are two affected projects:
crashing project.zip

By unpacking "pentatonica emitonica-01.mmpz" using this tool you can see that a peakcontroller is present but no peakcontrollereffect is found, as opposed to a good project that will show both the controller and the fx.

Here are the xml of one of the affected project, along with a sample .mmp file with peak controller present
goodvsbad.zip

@DeRobyJ
Copy link
Contributor Author

DeRobyJ commented Dec 19, 2017

Removing the peakcontroller line in <controllers>...</controllers> will restore the project.

@DeRobyJ
Copy link
Contributor Author

DeRobyJ commented Dec 19, 2017

Reproduced on RC5
Steps:

  • Create a new project
  • Add a peak controller on an instrument track on the BB-editor
  • Delete every BB-track on Song Editor, but don't touch the track with the peak controller

When saving, LMMS will delete every track in BB-editor because it's not needed, but it won't delete the peak controller, which won't be able to load!

@tresf tresf added the bug label Dec 19, 2017
@tresf tresf added this to the 1.2.0 milestone Dec 19, 2017
@tresf
Copy link
Member

tresf commented Dec 19, 2017

Temporary workaround for those wondering how @DeRobyJ was able to remove the <controllers> section, the general syntax is

lmms -d myproject.mmpz > myproject_xml.mmp

For a Windows user, something like this, perhaps:

"C:\Program Files\LMMS\lmms.exe" -d "%userprofile%\Desktop\myproject.mmpz" > "%userprofile%\Desktop\myproject_xml.mmp"

Or Apple:

"/Applications/LMMS.app/Contents/MacOS/lmms" -d "~\Desktop\myproject.mmpz" > "~\Desktop\myproject_xml.mmp"

Or Linux:

"~/Downloads/lmms-1.2.0-rc5-linux-x86_64.AppImage" -d "~\Desktop\myproject.mmpz" > "~\Desktop\myproject_xml.mmp"

Then you can edit the xml file according to @DeRobyJ's instructions and it will open just fine again.

@PhysSong
Copy link
Member

Related: #4029
Fixing #4029 will stop producing such project files, but There's still a issue with previous project file.

@musikBear
Copy link

@DeRobyJ

Some projects saved with 1.2 beta versions

What about 1.1.3 Does files saved with 1.1.3 also have this bug?
I do not have a 1.1.3 saved file with peak-controller inserted (afair), but it would be bad if old 1.1.3 ao even older projects with peakcontrollers fails.
If it is 'only' files from 1.2beta, that is affected, then imo: tuff-luck
Betas are not production versions, and a manual fix exists.
Imo, the crucial point is : What is the behaviour on official versions

@Umcaruje
Copy link
Member

Imo, the crucial point is : What is the behaviour on official versions

Absolutely not, the RC's are the current versions of the software and all bugs should be tested and be reported according to them, not 1.1.3. Not to mention all these versions are official because they are released on github and shared through the official website.

@tresf
Copy link
Member

tresf commented Dec 20, 2017

Imo, the crucial point is : What is the behaviour on official versions

@musikBear we sent him here and asked him to open this bug report. It's a critical bug for stable-1.2. Please stop policing the tracker. ❤️

@PhysSong
Copy link
Member

lmms/src/core/Song.cpp

Lines 1291 to 1294 in eb9b460

Controller * c = Controller::create( node.toElement(), this );
Q_ASSERT( c != NULL );
addController( c );

This crashes LMMS, even for release build. Defining QT_NO_DEBUG will suppress such crashes, but it isn't a real fix.
LMMS should be able to cleanup broken connection after loading bad project files.

@michaelgregorius
Copy link
Contributor

Copying my two comments from #2948 here for completeness (with one slight adjustment in the code snippet).

The problem with the file "0036 (lento)-01.mmpz" is caused by a non-existent effect that's referenced by a peak controller. The relevant section looks as follows:

<controllers>
  <Peakcontroller type="3" name="Controller 1" effectId="12959"/>
</controllers>

However, there is no effect with that ID and the string "12959" only shows up in the peak controller section throughout the whole file. This leads to a nullptr in Song::restoreControllerStates which triggers an assertion.

When I change the code as follows the file can at least be loaded:

void Song::restoreControllerStates( const QDomElement & element )
{
	QDomNode node = element.firstChild();
	while( !node.isNull() && !isCancelled() )
	{
		Controller * c = Controller::create( node.toElement(), this );

		if (c)
		{
			addController( c );
		}

		node = node.nextSibling();
	}
}

Although I assume that the peak controller will not be connected in that case. But it might come in handy to save most of the project.

@DeRobyJ
Copy link
Contributor Author

DeRobyJ commented Dec 21, 2017

Yes that will save old project, and there is nothing more to be done about it.
As for the bug itself, the problem is in #4029. Since the BBeditor tracks aren't present in the saved project, we can't have them restored.

@PhysSong
Copy link
Member

#4060 (comment) fixes the crash, but it messes up controller connections. I think I can fix it.

@PhysSong
Copy link
Member

PhysSong commented Jun 3, 2018

Pull request ready: #4391.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants