Patch for crash on exit
Moderator: Committer
Patch for crash on exit
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.
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.
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13587
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Patch for crash on exit
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...
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...
Re: Patch for crash on exit
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.
Re: Patch for crash on exit
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)
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.
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13587
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Patch for crash on exit
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.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)
- 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.
Re: Patch for crash on exit
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.
Re: Patch for crash on exit
Should that fix be merged to the release branch?
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13587
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Patch for crash on exit
If you test on OSX and it doesn't cause any apparent problems, sure.
Re: Patch for crash on exit
Done and committed.Geoff the Medio wrote:If you test on OSX and it doesn't cause any apparent problems, sure.
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...
- adrian_broher
- Programmer
- Posts: 1156
- Joined: Fri Mar 01, 2013 9:52 am
- Location: Germany
Re: Patch for crash on exit
I'm getting:
So it's probably the same issue as with the sources. So releasing the buffers should fix this.$ ./freeorion
FreeOrion server waiting for network events
mainSetupAndRunOgre caught CleanQuit
AL lib: (WW) FreeDevice: (0x16629d0) Deleting 6 Buffer(s)
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz
Re: Patch for crash on exit
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.adrian_broher wrote:I'm getting:
So it's probably the same issue as with the sources. So releasing the buffers should fix this.$ ./freeorion
FreeOrion server waiting for network events
mainSetupAndRunOgre caught CleanQuit
AL lib: (WW) FreeDevice: (0x16629d0) Deleting 6 Buffer(s)
- 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.
- adrian_broher
- Programmer
- Posts: 1156
- Joined: Fri Mar 01, 2013 9:52 am
- Location: Germany
Re: Patch for crash on exit
Thanks, the patch works as intended. Commited as revision 7498.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.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz
- adrian_broher
- Programmer
- Posts: 1156
- Joined: Fri Mar 01, 2013 9:52 am
- Location: Germany
Re: Patch for crash on exit
Committed sound_temp_disable.patch with r7552.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz