Patch for crash on exit

Programmers discuss here anything related to FreeOrion programming. Primarily for the developers to discuss.

Moderator: Committer

Post Reply
Message
Author
cataclysm
Space Krill
Posts: 6
Joined: Mon Aug 11, 2014 1:32 am

Patch for crash on exit

#1 Post by cataclysm »

Running on Linux, I was getting a seg fault in OpenAL when exiting. The sound_exit_cleanup.patch adds cleanup code to the Sound destructor to fix it.

I also noted the strange use of a std::stack<bool> to keep track of temporary sound disabling. Only true values are pushed on the stack, so all that matters is the size of the stack. sound_temp_disable.patch replaces the stack with a simple integer variable.

There are a number of other minor issues regarding error handling and code duplication. I'm going see what I can do to clean it up next.
Attachments

[The extension patch has been deactivated and can no longer be displayed.]

[The extension patch has been deactivated and can no longer be displayed.]

Any code of mine in this post is released under the GPL 2.0 or later.

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Patch for crash on exit

#2 Post by Geoff the Medio »

Please provide SVN patches, not git diffs. Some discussion about this is here: viewtopic.php?p=70540#p70540

That said, these are probably simple enough to test by hand...

Edit: The sound cleanup on exit runs fine on my Windows build test, but I didn't have the issue anyway... If someone else on Linux who has a similar issue could test, that would be helpful...

cataclysm
Space Krill
Posts: 6
Joined: Mon Aug 11, 2014 1:32 am

Re: Patch for crash on exit

#3 Post by cataclysm »

Sorry, I was unaware that git diffs were difficult to apply for subversion users. I'm attaching new versions of the files created with svn.
Attachments

[The extension patch has been deactivated and can no longer be displayed.]

[The extension patch has been deactivated and can no longer be displayed.]

Any code of mine in this post is released under the GPL 2.0 or later.

Mitten.O
Programmer
Posts: 255
Joined: Sun Apr 06, 2014 4:15 pm

Re: Patch for crash on exit

#4 Post by Mitten.O »

I am on Ubuntu 14.04 and didn't have a segfault, but did have a message warning that some device was left open.
With these patches, I have a message telling that AL lib is cleaning up, which is an improvement.
No message would be even cleaner, though, depending on what this one means.

Before:
AL lib: ReleaseALC: 1 device not closed

Now:
AL lib: FreeContext: (0x1b34120) Deleting 16 Source(s)
Any code by me in this post is released under GPL 2.0 or later.

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Patch for crash on exit

#5 Post by Geoff the Medio »


cataclysm
Space Krill
Posts: 6
Joined: Mon Aug 11, 2014 1:32 am

Re: Patch for crash on exit

#6 Post by cataclysm »

Mitten.O wrote:I am on Ubuntu 14.04 and didn't have a segfault, but did have a message warning that some device was left open.
With these patches, I have a message telling that AL lib is cleaning up, which is an improvement.
No message would be even cleaner, though, depending on what this one means.

Before:
AL lib: ReleaseALC: 1 device not closed

Now:
AL lib: FreeContext: (0x1b34120) Deleting 16 Source(s)
The OpenAL documentation states that "All sources within a context will automatically be deleted during context destruction." so I don't think this indicates any problem. However, we can delete them manually with the attached patch. Try it and see if that clears up your exit messages.
Attachments

[The extension patch has been deactivated and can no longer be displayed.]

Any code of mine in this post is released under the GPL 2.0 or later.

Mitten.O
Programmer
Posts: 255
Joined: Sun Apr 06, 2014 4:15 pm

Re: Patch for crash on exit

#7 Post by Mitten.O »

Yes. That clears it entirely. Even if there was no real problem, it is nicer this way.
Any code by me in this post is released under GPL 2.0 or later.

User avatar
Vezzra
Release Manager, Design
Posts: 6095
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: Patch for crash on exit

#8 Post by Vezzra »

Should that fix be merged to the release branch?

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Patch for crash on exit

#9 Post by Geoff the Medio »

If you test on OSX and it doesn't cause any apparent problems, sure.

User avatar
Vezzra
Release Manager, Design
Posts: 6095
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: Patch for crash on exit

#10 Post by Vezzra »

Geoff the Medio wrote:If you test on OSX and it doesn't cause any apparent problems, sure.
Done and committed.

What about the delete_sound_sources patch? Is that going to be committed, and if, should it also go into the release branch? If yes, that should happen within our deadline for RC2...

User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: Patch for crash on exit

#11 Post by adrian_broher »

I'm getting:
$ ./freeorion
FreeOrion server waiting for network events
mainSetupAndRunOgre caught CleanQuit
AL lib: (WW) FreeDevice: (0x16629d0) Deleting 6 Buffer(s)
So it's probably the same issue as with the sources. So releasing the buffers should fix this.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

cataclysm
Space Krill
Posts: 6
Joined: Mon Aug 11, 2014 1:32 am

Re: Patch for crash on exit

#12 Post by cataclysm »

adrian_broher wrote:I'm getting:
$ ./freeorion
FreeOrion server waiting for network events
mainSetupAndRunOgre caught CleanQuit
AL lib: (WW) FreeDevice: (0x16629d0) Deleting 6 Buffer(s)
So it's probably the same issue as with the sources. So releasing the buffers should fix this.
Here's the patch that deletes the buffers. You need to verify that it clears the message since the message didn't show up on my machine.
Attachments

[The extension patch has been deactivated and can no longer be displayed.]

Any code of mine in this post is released under the GPL 2.0 or later.

User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: Patch for crash on exit

#13 Post by adrian_broher »

cataclysm wrote:Here's the patch that deletes the buffers. You need to verify that it clears the message since the message didn't show up on my machine.
Thanks, the patch works as intended. Commited as revision 7498.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: Patch for crash on exit

#14 Post by adrian_broher »

Committed sound_temp_disable.patch with r7552.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

Post Reply