Menu
▾
▴
[Gaim-patches] [ gaim-Patches-1191553 ] [MSN] msn_p2p_msg() crash fix
|
From: SourceForge.net <no...@so...> - 2005-04-29 06:47:49
|
Patches item #1191553, was opened at 2005-04-28 03:28 Message generated for change (Comment added) made by evands You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=1191553&group_id=235 Category: None Group: None >Status: Closed >Resolution: Out of Date Priority: 5 Submitted By: Evan Schoenberg (evands) Assigned to: Stu Tomlinson (nosnilmot) Summary: [MSN] msn_p2p_msg() crash fix Initial Comment: This patch adds g_return_if_fail() safety for the arguments to msn_p2p_msg(), bringing it in line with other msn functions and hopefully fixing a crash in the process. It also corrects a comment typo, "its" rather than "it's". I see a lot of crashes in our crash reporter from MSN which look like: 0 Libgaim 0x0497b374 msn_p2p_msg 0x3c 1 Libgaim 0x049771f8 msg_cmd_post 0x64 2 Libgaim 0x049972a8 read_cb 0x1cc and usually have descriptions of having been idling and come back to find a crash. I haven't been able to reproduce this crash with the patch, though I couldn't reproduce it reliably before, either. It applies cleanly against oldstatus and HEAD. ---------------------------------------------------------------------- >Comment By: Evan Schoenberg (evands) Date: 2005-04-29 01:47 Message: Logged In: YES user_id=669684 Closing this since a "fix" is in CVS now. ---------------------------------------------------------------------- Comment By: Evan Schoenberg (evands) Date: 2005-04-28 15:15 Message: Logged In: YES user_id=669684 Heh, so I protected everything except the actual crash, it looks like? cmdproc->data is returning NULL? ---------------------------------------------------------------------- Comment By: Daniel Atallah (datallah) Date: 2005-04-28 14:48 Message: Logged In: YES user_id=325843 Line numbers (on win32) are available in the crash report on bug# 1183272 (and the many duplicates of it) https://sourceforge.net/tracker/?func=detail&atid=100235&aid=1183272&group_id=235 ---------------------------------------------------------------------- Comment By: Stu Tomlinson (nosnilmot) Date: 2005-04-28 14:32 Message: Logged In: YES user_id=309779 In Gaim, whenever a g_return_if_fail is encountered, a log message is automatically generated, eg: g_log: file blist.c: line 2577 (gaim_blist_node_get_string): assertion `setting->type == GAIM_BLIST_NODE_SETTING_STRING' failed Setting a breakpoint on g_log is sometimes useful in such cases, although in this case it probably won't help all that much (we already have most of the backtrace from the crash reports). A debug log (I'm not sure if Adium implements this in the same way as Gaim, the equivalent of the output from running gaim -d) might be useful in providing some insight into what was going on just prior to the crash in helping isolate the real bug. FWIW I believe this is caused by interacting in some way with users of MSN 7 (but I too have been unable to reproduce it). As for improving debug information, gcc uses the -g option to build in more debugging information, -g2 and -g3 include line number information (according to man gcc). I'm not sure how useful this will be to your crash reporter though. If you strip the binaries some or all debugging info will be lost. ---------------------------------------------------------------------- Comment By: Evan Schoenberg (evands) Date: 2005-04-28 14:31 Message: Logged In: YES user_id=669684 I'm not sure I'm ever going to see this one locally, actually... I don't even use MSN, so I don't have any MSN contacts using MSN7... msn_p2p_msg is from new msn clients, right? I just assume that because it's a fairly new issue so far as I can tell. So I've got debug logging in place.. but a heavy MSN user might need to actually determine the source of this. ---------------------------------------------------------------------- Comment By: Evan Schoenberg (evands) Date: 2005-04-28 14:17 Message: Logged In: YES user_id=669684 It's certainly covering up a deeper issue... but I can't find what that issue would be. But it's definitely a common crash... and as mentioned above, it is reported as being effectively random. I can't reproduce it at all : I'll some gaim_debug() logging if it's about to return because of a failure and see which if any are getting hit. My gdb knowledge isn't amazing... breakpointing on g_return_if_fail is well-nigh useless since it breaks every time the function gets called. Is there a better way? Do you have any suggestions on compiler settings which would leave line numbers rather than just symbols intact? I don't know how to make that happen, but would love to be able to distribute betas which did it as it'd be insanely useful in all sorts of situations. ---------------------------------------------------------------------- Comment By: Stu Tomlinson (nosnilmot) Date: 2005-04-28 08:30 Message: Logged In: YES user_id=309779 I'm hesitant to accept this without any confirmation that it does anything useful at all. There are a number of similar bugs reported, but they all indicate that slplink->swboard is either NULL or has been free'd at slp.c:738 - and I don't think this patch would affect those situations. Additionally, using g_return_if_fail like this is quite possibly covering up some deeper bug which could just end up causing crashes or other problems elsewhere. In your testing with the patch did you notice whether any of the g_return_if_fails were actually being hit? Do you have *any* pointers on how to reproduce the crash? Can you get your crash reporter to include line numbers? :) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=1191553&group_id=235 |
