From 87835fd84193bfa1bd42f9f15fd69b2aebd276ae Mon Sep 17 00:00:00 2001 From: Bryce Denney Date: Wed, 25 Sep 2002 22:54:23 +0000 Subject: [PATCH] - fix memory leaks in the text mode interface that I introduced while working on the wxWindows interface. There are many more changes here than absolutely required to fix the memory leaks. Instead, I've tried to clean things up so that it does the right thing, and is easier to read and maintain. - For events that the text mode interface is going to ignore anyway, I #ifdefed the event creation code instead of calling new and then delete. - now all synchronous events in siminterface.cc are created as local variables on the stack. Some of them were allocated with new before, and yes some of them leaked. - now I ignore the result of sim_to_ci_event (&event). It was always returning a pointer to the input event anyway. This makes the event sending code simpler. - wxmain.cc: - in the BxEvent handling functions, now all cases "break" down to common code at the end which deletes async events. This is easier to read than having each case handle the delete individually. - in OnLogMsg, do not delete the event here because it is now handled in the common code of OnSim2CIEvent instead. - thanks to Christophe for pointing out the location of the worst memory leak. --- bochs/gui/control.cc | 7 +++-- bochs/gui/siminterface.cc | 59 ++++++++++++++++++++------------------- bochs/gui/wxmain.cc | 53 +++++++++++++++++------------------ 3 files changed, 59 insertions(+), 60 deletions(-) diff --git a/bochs/gui/control.cc b/bochs/gui/control.cc index c6b009cb8..9b21ee884 100644 --- a/bochs/gui/control.cc +++ b/bochs/gui/control.cc @@ -1,5 +1,5 @@ ///////////////////////////////////////////////////////////////////////// -// $Id: control.cc,v 1.62 2002-09-22 20:56:11 cbothamy Exp $ +// $Id: control.cc,v 1.63 2002-09-25 22:54:22 bdenney Exp $ ///////////////////////////////////////////////////////////////////////// // // This is code for a text-mode configuration interfac. Note that this file @@ -672,8 +672,9 @@ ask: } return event; case BX_ASYNC_EVT_REFRESH: - // ignore refresh events - return event; + case BX_ASYNC_EVT_DBG_MSG: + // The text mode interface does not use these events, so I commented + // out the code that produces them in siminterface.cc. default: fprintf (stderr, "Control panel: notify callback called with event type %04x\n", event->type); return event; diff --git a/bochs/gui/siminterface.cc b/bochs/gui/siminterface.cc index cede416c1..5fea78ad5 100644 --- a/bochs/gui/siminterface.cc +++ b/bochs/gui/siminterface.cc @@ -1,5 +1,5 @@ ///////////////////////////////////////////////////////////////////////// -// $Id: siminterface.cc,v 1.68 2002-09-24 17:57:48 bdenney Exp $ +// $Id: siminterface.cc,v 1.69 2002-09-25 22:54:22 bdenney Exp $ ///////////////////////////////////////////////////////////////////////// // // See siminterface.h for description of the siminterface concept. @@ -439,14 +439,14 @@ bx_real_sim_c::sim_to_ci_event (BxEvent *event) int bx_real_sim_c::log_msg (const char *prefix, int level, const char *msg) { - BxEvent *be = new BxEvent (); - be->type = BX_SYNC_EVT_LOG_ASK; - be->u.logmsg.prefix = prefix; - be->u.logmsg.level = level; - be->u.logmsg.msg = msg; + BxEvent be; + be.type = BX_SYNC_EVT_LOG_ASK; + be.u.logmsg.prefix = prefix; + be.u.logmsg.level = level; + be.u.logmsg.msg = msg; //fprintf (stderr, "calling notify.\n"); - BxEvent *response = sim_to_ci_event (be); - return response? response->retcode : -1; + sim_to_ci_event (&be); + return be.retcode; } // Called by simulator whenever it needs the user to choose a new value @@ -459,11 +459,11 @@ bx_real_sim_c::ask_param (bx_id param) bx_param_c *paramptr = SIM->get_param(param); BX_ASSERT (paramptr != NULL); // create appropriate event - BxEvent *event = new BxEvent (); - event->type = BX_SYNC_EVT_ASK_PARAM; - event->u.param.param = paramptr; - BxEvent *response = sim_to_ci_event (event); - return response->retcode; + BxEvent event; + event.type = BX_SYNC_EVT_ASK_PARAM; + event.u.param.param = paramptr; + sim_to_ci_event (&event); + return event.retcode; } int @@ -477,8 +477,7 @@ bx_real_sim_c::ask_filename (char *filename, int maxlen, char *prompt, char *the param.get_options()->set (flags); event.type = BX_SYNC_EVT_ASK_PARAM; event.u.param.param = ¶m; - BxEvent *response = sim_to_ci_event (&event); - BX_ASSERT ((response == &event)); + sim_to_ci_event (&event); if (event.retcode >= 0) memcpy (filename, param.getptr(), maxlen); return event.retcode; @@ -489,13 +488,10 @@ bx_real_sim_c::periodic () { // give the GUI a chance to do periodic things on the bochs thread. in // particular, notice if the thread has been asked to die. - BxEvent *tick = new BxEvent (); - tick->type = BX_SYNC_EVT_TICK; - BxEvent *response = sim_to_ci_event (tick); - int retcode = response->retcode; - BX_ASSERT (response == tick); - delete tick; - if (retcode < 0) { + BxEvent tick; + tick.type = BX_SYNC_EVT_TICK; + sim_to_ci_event (&tick); + if (tick.retcode < 0) { BX_INFO (("Bochs thread has been asked to quit.")); bx_atexit (); quit_sim (0); @@ -577,10 +573,14 @@ bx_real_sim_c::create_disk_image ( } void bx_real_sim_c::refresh_ci () { - BxEvent *refresh = new BxEvent (); - refresh->type = BX_ASYNC_EVT_REFRESH; - sim_to_ci_event (refresh); - // the event will be freed by the recipient +#if BX_WITH_WX + // presently, only wxWindows interface uses these events + // It's an async event, so allocate a pointer and send it. + // The event will be freed by the recipient. + BxEvent *event = new BxEvent (); + event->type = BX_ASYNC_EVT_REFRESH; + sim_to_ci_event (event); +#endif } bx_param_c * @@ -624,9 +624,8 @@ char *bx_real_sim_c::debug_get_next_command () BxEvent event; event.type = BX_SYNC_EVT_GET_DBG_COMMAND; BX_INFO (("asking for next debug command")); - BxEvent *response = sim_to_ci_event (&event); + sim_to_ci_event (&event); BX_INFO (("received next debug command: '%s'", event.u.debugcmd.command)); - BX_ASSERT ((response == &event)); if (event.retcode >= 0) return event.u.debugcmd.command; return NULL; @@ -635,12 +634,14 @@ char *bx_real_sim_c::debug_get_next_command () void bx_real_sim_c::debug_puts (const char *text) { #if BX_WITH_WX - // send message to the GUI + // send message to the wxWindows debugger BxEvent *event = new BxEvent (); event->type = BX_ASYNC_EVT_DBG_MSG; event->u.logmsg.msg = text; sim_to_ci_event (event); + // the event will be freed by the recipient #else + // text mode debugger: just write to console fputs (text, stderr); #endif } diff --git a/bochs/gui/wxmain.cc b/bochs/gui/wxmain.cc index b1b896b8f..45c05f31e 100644 --- a/bochs/gui/wxmain.cc +++ b/bochs/gui/wxmain.cc @@ -1,5 +1,5 @@ ///////////////////////////////////////////////////////////////// -// $Id: wxmain.cc,v 1.58 2002-09-25 19:05:01 bdenney Exp $ +// $Id: wxmain.cc,v 1.59 2002-09-25 22:54:23 bdenney Exp $ ///////////////////////////////////////////////////////////////// // // wxmain.cc implements the wxWindows frame, toolbar, menus, and dialogs. @@ -188,20 +188,26 @@ MyApp::DefaultCallback2 (BxEvent *event) wxString text; text.Printf ("Error: %s", event->u.logmsg.msg); wxMessageBox (text, "Error", wxOK | wxICON_ERROR ); - //theFrame->OnLogMsg (event); + // maybe I can make OnLogMsg display something that looks appropriate. + // theFrame->OnLogMsg (event); event->retcode = BX_LOG_ASK_CHOICE_CONTINUE; // There is only one thread at this point. if I choose DIE here, it will // call fatal() and kill the whole app. break; } case BX_ASYNC_EVT_REFRESH: - case BX_SYNC_EVT_ASK_PARAM: case BX_ASYNC_EVT_DBG_MSG: + break; // ignore + case BX_SYNC_EVT_ASK_PARAM: case BX_SYNC_EVT_GET_DBG_COMMAND: - break; + break; // ignore default: wxLogDebug ("unknown event type %d", event->type); } + if (BX_EVT_IS_ASYNC(event-type)) { + delete event; + event = NULL; + } return event; } @@ -1126,30 +1132,27 @@ MyFrame::OnSim2CIEvent (wxCommandEvent& event) // response. async event handlers MUST delete the event. switch (be->type) { case BX_ASYNC_EVT_REFRESH: - delete be; RefreshDialogs (); - return; + break; case BX_SYNC_EVT_ASK_PARAM: wxLogDebug ("before HandleAskParam"); be->retcode = HandleAskParam (be); wxLogDebug ("after HandleAskParam"); - // sync must return something; just return a copy of the event. + // return a copy of the event back to the sender. sim_thread->SendSyncResponse(be); wxLogDebug ("after SendSyncResponse"); - return; + break; #if BX_DEBUGGER case BX_ASYNC_EVT_DBG_MSG: showDebugLog->AppendText (be->u.logmsg.msg); // free the char* which was allocated in dbg_printf delete [] ((char*) be->u.logmsg.msg); - // free the whole event - delete be; - return; + break; #endif case BX_SYNC_EVT_LOG_ASK: case BX_ASYNC_EVT_LOG_MSG: OnLogMsg (be); - return; + break; case BX_SYNC_EVT_GET_DBG_COMMAND: wxLogDebug ("BX_SYNC_EVT_GET_DBG_COMMAND received"); if (debugCommand == NULL) { @@ -1172,21 +1175,18 @@ MyFrame::OnSim2CIEvent (wxCommandEvent& event) be->retcode = 1; sim_thread->SendSyncResponse (be); } - return; + break; default: wxLogDebug ("OnSim2CIEvent: event type %d ignored", (int)be->type); - if (BX_EVT_IS_ASYNC(be->type)) { - delete be; - } else { - // assume it's a synchronous event and send back a response, to avoid - // potential deadlock. + if (!BX_EVT_IS_ASYNC(be->type)) { + // if it's a synchronous event, and we fail to send back a response, + // the sim thread will wait forever. So send something! sim_thread->SendSyncResponse(be); } - return; + break; } - // it is critical to send a response back eventually since the sim thread - // is blocking. - wxASSERT_MSG (0, "switch stmt should have returned"); + if (BX_EVT_IS_ASYNC(be->type)) + delete be; } void MyFrame::OnLogMsg (BxEvent *be) { @@ -1194,13 +1194,10 @@ void MyFrame::OnLogMsg (BxEvent *be) { be->u.logmsg.level, be->u.logmsg.prefix, be->u.logmsg.msg); - if (be->type == BX_ASYNC_EVT_LOG_MSG) { - // don't ask for user response - delete be; // because it was async - return; - } else { + if (be->type == BX_ASYNC_EVT_LOG_MSG) + return; // we don't have any place to display log messages + else wxASSERT (be->type == BX_SYNC_EVT_LOG_ASK); - } wxString levelName (SIM->get_log_level_name (be->u.logmsg.level)); LogMsgAskDialog dlg (this, -1, levelName); // panic, error, etc. #if !BX_DEBUGGER