From 766c46166c26b46ac25cac0b06c8caab76a36cc7 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Sat, 8 Mar 2008 00:18:51 +0000 Subject: [PATCH] Team debug effort with Michael Lotz: Tracked down the root cause of the issue worked around in r24228. The behavior that was occurring in this case was as follows: Vision created a BInvoker off the heap, containing a BMessage likewise off the heap. It then called BAlert::Go with this invoker. When any of the buttons in the BAlert were pushed, _SendMessage would enqueue the message into the target looper (in this case, Vision's window) and wake it up with write_port_etc. However, in some cases this would result in a reschedule such that Vision's looper executed before _SendMessage() was able to complete all its processing. Upon receiving this message, Vision destroyed the BInvoker in question, which in turn deleted its owned BMessage -- which was the message that was still in the middle of _SendMessage()!. Consequently it would crash a few lines later when it hit IsSourceWaiting() and tried to check flags off its header off the already destroyed this. To fix this, we now do the AddMessage/write_port_etc last before returning. Also removed the sanity checks added in r24228 so we don't mask other problems of similar nature. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24297 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/kits/app/Message.cpp | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/kits/app/Message.cpp b/src/kits/app/Message.cpp index 7828cd1e8f..6d64d1a74a 100644 --- a/src/kits/app/Message.cpp +++ b/src/kits/app/Message.cpp @@ -613,7 +613,7 @@ bool BMessage::IsSourceWaiting() const { DEBUG_FUNCTION_ENTER; - return fHeader && (fHeader->flags & MESSAGE_FLAG_REPLY_REQUIRED) + return (fHeader->flags & MESSAGE_FLAG_REPLY_REQUIRED) && !(fHeader->flags & MESSAGE_FLAG_REPLY_DONE); } @@ -622,7 +622,7 @@ bool BMessage::IsSourceRemote() const { DEBUG_FUNCTION_ENTER; - return fHeader && (fHeader->flags & MESSAGE_FLAG_WAS_DELIVERED) + return (fHeader->flags & MESSAGE_FLAG_WAS_DELIVERED) && (fHeader->reply_team != BPrivate::current_team()); } @@ -1953,21 +1953,7 @@ BMessage::_SendMessage(port_id port, team_id portOwner, int32 token, header->reply_target = replyToPrivate.Token(); header->flags |= MESSAGE_FLAG_WAS_DELIVERED; - if (direct != NULL) { - KTRACE("BMessage send direct: port: %ld, token: %ld, " - "message: '%c%c%c%c'", port, token, - char(what >> 24), char(what >> 16), char(what >> 8), (char)what); - - // this is a local message transmission - direct->AddMessage(copy); - - if (direct->Queue()->IsNextMessage(copy) && port_count(port) <= 0) { - // there is currently no message waiting, and we need to wakeup the looper - write_port_etc(port, 0, NULL, 0, B_RELATIVE_TIMEOUT, 0); - } - - direct->Release(); - } else { + if (direct == NULL) { KTRACE("BMessage send remote: team: %ld, port: %ld, token: %ld, " "message: '%c%c%c%c'", portOwner, port, token, char(what >> 24), char(what >> 16), char(what >> 8), (char)what); @@ -1984,6 +1970,24 @@ BMessage::_SendMessage(port_id port, team_id portOwner, int32 token, fHeader->flags |= MESSAGE_FLAG_REPLY_DONE; } + // we need to do this last because it is possible our + // message might be destroyed after it's enqueued in the + // target looper. Thus we don't want to do any ops that depend on + // members of this after the enqueue. + if (direct != NULL) { + KTRACE("BMessage send direct: port: %ld, token: %ld, " + "message: '%c%c%c%c'", port, token, + char(what >> 24), char(what >> 16), char(what >> 8), (char)what); + + // this is a local message transmission + direct->AddMessage(copy); + if (direct->Queue()->IsNextMessage(copy) && port_count(port) <= 0) { + // there is currently no message waiting, and we need to wakeup the looper + write_port_etc(port, 0, NULL, 0, B_RELATIVE_TIMEOUT, 0); + } + direct->Release(); + } + free(buffer); return result; }