wesnoth crashed with SIGSEGV in malloc()

[DrMcAwesome]

Binary package hint: wesnoth

loaded an add-on successfully. Went back into the menu to add a second add-on and Wesnoth crashed. Currently using Intrepid Ibex with latest version of Wesnoth.

ProblemType: Crash
Architecture: i386
DistroRelease: Ubuntu 8.10
ExecutablePath: /usr/games/wesnoth
Package: wesnoth 1:1.4.5-1
ProcAttrCurrent: unconfined
ProcCmdline: wesnoth
ProcEnviron:
 PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=en_US.UTF-8
 SHELL=/bin/bash
Signal: 11
SourcePackage: wesnoth
Stacktrace:
 #0 0xb79eabb1 in ?? () from /lib/tls/i686/cmov/libc.so.6
 #1 0xb79ec865 in malloc () from /lib/tls/i686/cmov/libc.so.6
 #2 0xb7ced09b in ?? () from /usr/lib/libfribidi.so.0
 Backtrace stopped: previous frame inner to this frame (corrupt stack?)
StacktraceTop:
 ?? () from /lib/tls/i686/cmov/libc.so.6
 malloc () from /lib/tls/i686/cmov/libc.so.6
 ?? () from /usr/lib/libfribidi.so.0
Title: wesnoth crashed with SIGSEGV in malloc()
Uname: Linux 2.6.27-7-generic i686
UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare

[DrMcAwesome (scott-flowers)]

Wesnoth crashed after winning a scenario.

[Apport retracing service (apport)]

StacktraceTop:_int_malloc () from /lib/tls/i686/cmov/libc.so.6
malloc () from /lib/tls/i686/cmov/libc.so.6
fribidi_analyse_string () from /usr/lib/libfribidi.so.0
fribidi_analyse_string () from /usr/lib/libfribidi.so.0
?? ()

[Pauli (paniemin)]

For me this seems a lot like crash could happen because of race condition in network threading code. (Happened for me when couln't connect to addons server 3 times in a row)

calgrind report would be usefull if anyone wants to debug

[Rhonda D'Vine (rhonda)]

Could you try if this still happens with the version in jaunty for you? There were quite some bugfixes incorporated since and no feature changes, it's still 1.4 compatible version.

[Pauli (paniemin)]

I checked 1.4.7 with valgrind (calgrind was just a typo in previous post) and there is no trace of race condition in disconnect any more.

Solution to this might be either to push 1.4.7 to intrepid or backport network_worker.cpp. I don't know which one is preferred but both should fix the crash problem (that has about 25 % chance to happen in my system while disconnecting)

[Dara Adib (daradib)]

Marking as fix released as this bug is fixed in the latest development release of Ubuntu (Jaunty). I would recommend filing a backport request to Ubuntu 8.10 and possibly Ubuntu 8.04 following these instructions: https://help.ubuntu.com/community/UbuntuBackports#How%20to%20request%20new%20packages

[Emilio Pozuelo Monfort (pochu)]

Not really. We want a Stable Release Update for a crasher, not a backport. Task opened, will try to look into this soonish, but if you can provide a diffstat and a diff of network_worker.cpp that would be nice.

Gerfried, do you think it's reasonably safe to put 1.4.7 into Intrepid, or we rather cherry-pick the fix for this?

[Pauli (paniemin)]

Fix for this crash is just a few lines change:
http://svn.gna.org/viewcvs/wesnoth?rev=29514&dir_pagestart=100&view=rev

I don't know much what has changed in 1.4.x. But I think for wesnoth it would be reasonable to always upgrade to the latest stable version. Code wise it there is only bug fixes but all new graphics are ported to stable branch which adds huge amount changes to the stable branch.

[Emilio Pozuelo Monfort (pochu)]

Thanks Pauli! I've prepared an SRU for this. MOTU SRU, please confirm.

[Cody A.W. Somerville (cody-somerville)]

ACK

[Emilio Pozuelo Monfort (pochu)]

Uploaded, pending approval from an archive admin.

[Martin Pitt (pitti)]

Accepted into intrepid-proposed, please test and give feedback here. Please see https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Rhonda D'Vine (rhonda)]

* Emilio Pozuelo Monfort <email address hidden> [2009-01-27 22:41:18 CET]:
> Not really. We want a Stable Release Update for a crasher, not a
> backport. Task opened, will try to look into this soonish, but if you
> can provide a diffstat and a diff of network_worker.cpp that would be
> nice.
>
> Gerfried, do you think it's reasonably safe to put 1.4.7 into Intrepid,
> or we rather cherry-pick the fix for this?

 I haven't done a direct diff, but usually wesnoth has strict guidelines
for their stable updates: There won't be any feature changes at all
because a stable release has to be compatible with every other patch
update to it. So, most of the times, the only changes are to the
translation files and bugfixes for crashes in campaigns or in gameplay
that don't expose a different behavior that would result in an OutOfSync
situation for multiplayer games (see LP #256345 for a story in this).

* Pauli <email address hidden> [2009-01-28 17:07:36 CET]:
> Fix for this crash is just a few lines change:
> http://svn.gna.org/viewcvs/wesnoth?rev=29514&dir_pagestart=100&view=rev

 Thanks for digging it up. :)

> I don't know much what has changed in 1.4.x.

 There's always the upstream changelog in the root source directory. :)

> But I think for wesnoth it would be reasonable to always upgrade to
> the latest stable version. Code wise it there is only bug fixes but
> all new graphics are ported to stable branch which adds huge amount
> changes to the stable branch.

 New graphics aren't that common but yeah, they happen. A stable release
isn't expected to look completely different than one before, and it
shouldn't.

 For convenience, the upstream changelog for 1.4.6 and 1.4.7:

#v+
Version 1.4.7:
 * campaigns:
   * Descent into Darkness
     * fixed bats crashing through the ice in 'Beginning of Revenge' (bug
         #12685)
   * Heir to the Throne:
     * Added an idle animation for L2 Princess.
 * graphics:
   * new or updated unit frames: Gryphon Rider, Gryphon Master
 * language and i18n:
   * updated translations: Danish, Finnish, Hungarian, Italian
 * miscellaneous and bug fixes:
   * replace a #elif without condition with #else (debian patch #505957)
   * fixed campaign replays (bug #12005)
   * start-of-scenario saves are properly marked as such

Version 1.4.6:
 * language and i18n:
   * updated translations: Czech, Danish, Finnish, French, Galician, German,
     Hungarian, Italian, Polish, Slovak, Turkish, Valencian
 * graphics
   * fixed the parts of the undead flags which were not TC'd
#v-

 I am not trying to convince you to push those in, especially since I
reworked a bit on the packaging end, too, especially in the 1.4.6-1
upload, even though that would close a LP bug:

#v+
  * New stable upstream release, bugfixes and translation updates:
    - Fixes assertion with End Turn (closes: #498154)
  * Lower wesnoth-music from Recommends to Suggests in wesnoth-data (LP:
  * #281791)
  * Rename wesnoth to wesnoth-core and wesnoth-all to wesnoth. Document
  * the
    switch in the NEWS file (closes: #500935)
  * Allow to hand over arguments to wesnoth-nolog.
  * Quote campaign names in package descriptions.
#v-

 So save bet is to just pick the bugf...

[Emilio Pozuelo Monfort (pochu)]

Gerfried Fuchs wrote:
> I haven't done a direct diff, but usually wesnoth has strict guidelines
> for their stable updates: There won't be any feature changes at all
> because a stable release has to be compatible with every other patch
> update to it. So, most of the times, the only changes are to the
> translation files and bugfixes for crashes in campaigns or in gameplay
> that don't expose a different behavior that would result in an OutOfSync
> situation for multiplayer games (see LP #256345 for a story in this).

I see. Definitely it's something to consider for the future. This time I just
backported the fix though.

[split]
> I am not trying to convince you to push those in, especially since I
> reworked a bit on the packaging end, too, especially in the 1.4.6-1
> upload, even though that would close a LP bug:

But that bug means renaming a binary package, which is a no-go for a stable
release. In any case, I'd update the package to 1.4.7 but using the same packaging.

> So save bet is to just pick the bugfix, even though I guess that some
> might disagree with it because technically it's "just" an upstream
> stable release update with pretty tight rules in that respect - to some
> degree possibly compareable to the postgresql updates within their 8.3
> branches and so on.

For that, a general permission needs to be given from the Technical Board (one
for the package, then every new upstream stable release can be uploaded), so
perhaps we can consider it for the future. Will have to think about it.

> Hope that deeper insight helps a bit. :)

It surely does, thanks a lot!

Best regards,
Emilio

[Jamie Strandboge (jdstrand)]

This upload will be superceded by security fixes in bug #336396. Once those fixes are in the archive, please regenerate your proposed package based the new security update. Thanks!

[Jamie Strandboge (jdstrand)]

Marking to Triaged per last comment.

[Alex Valavanis (valavanisalex)]

Intrepid Ibex reached end-of-life on 30 April 2010 so I am closing the report. The bug has been fixed in newer releases of Ubuntu.