Help needed for an Intuition event loop

[thyslo]

Hi all,
I’m relatively new to Amiga programming, nevertheless I choose a bigger project coded in c++ to start with it.

And now I’m messing around in the Intuition event loop without getting any further. Maybe one of you more advanced Amiga programmers see the fault and can give a hint?

My program has up to three windows opened. But at the start only one window is opened. When entering the loop only pWin3 is opened, pWin1 and pWin2 are NULL. Here’s the loop as it is by now:

 1 void Application::intuiEventLoop()
 2 {
 3   struct Window* pWin1 = m_pLeftWin->IntuiWindow();
 4   struct Window* pWin2 = m_pRightWin->IntuiWindow();
 5   struct Window* pWin3 = m_pOpenFilesWin->IntuiWindow();
 6   struct Menu* pMenu = m_pMenu->IntuiMenu();
 7 
 8   while(m_bExitRequested == false)
 9   {
10     // Waiting for messages from Intuition
11     Wait(1L << pWin1->UserPort->mp_SigBit |
12          1L << pWin2->UserPort->mp_SigBit |
13          1L << pWin3->UserPort->mp_SigBit);
14 
15     struct IntuiMessage* pMsg;
16 
17     while ((pMsg = (struct IntuiMessage *)GetMsg(pWin1->UserPort)) ||
18            (pMsg = (struct IntuiMessage *)GetMsg(pWin2->UserPort)) ||
19            (pMsg = (struct IntuiMessage *)GetMsg(pWin3->UserPort)) )
20     {
21       // Get all data we need from message
22       ULONG msgClass = pMsg->Class;
23       UWORD msgCode = pMsg->Code;
24       APTR msgIAddress = pMsg->IAddress;
25       struct Window* msgWindow = pMsg->IDCMPWindow;
26 
27       // When we're through with a message, reply
28       ReplyMsg((struct Message *)pMsg);
29 
30       if(msgClass == IDCMP_MENUPICK)
31       {
32         //
33         // Menupick messages are handled here
34         //
35         UWORD menuNumber = msgCode;
36         struct MenuItem* pSelectedItem = ItemAddress(pMenu, menuNumber);
37 
38         if(pSelectedItem != NULL)
39         {
40           // Getting the user data from selected menu item
41           APTR pUserData = GTMENUITEM_USERDATA(pSelectedItem);
42           if(pUserData != NULL)
43           {
44             // Our menu user data always contain a pointer to a Command
45             Command* pSelecedCommand = static_cast<Command*>(pUserData);
46 
47             // Execute this command
48             pSelecedCommand->Execute();
49           }
50         }
51       }
52       else
53       {
54         //
55         // All other messages are handled in the appropriate window
56         //
57         if(msgWindow == m_pLeftWin->IntuiWindow())
58         {
59           m_pLeftWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
60         }
61         else if(msgWindow == m_pRightWin->IntuiWindow())
62         {
63           m_pRightWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
64         }
65         else if(msgWindow == m_pOpenFilesWin->IntuiWindow())
66         {
67           m_pOpenFilesWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
68         }
69       }
70     }
71   }
72 }

The problem is, the program crashes if I close pWin3 by clicking on its close icon.

By clicking on the close icon the HandleIdcmp() method of the higher level c++ object encapsulation of pWin3 gets called. There inside when the close icon was clicked, CloseWindow() will be called. So after this the pointer pWin3 points to an address where no opened window exists anymore.

Can this be the reason for the crash?

Or is it the Wait() call where an ->UserPort->mp_SigBit of the NULL pointers is done, as pWin1 and pWin2 are NULL at the beginning.

But even if I comment out all pWin1 and pWin2 occurrences inside the loop the program crashes.

Is there a better way to handle multiple windows in a higher level event loop without making it too complex?

The complete src, coded in C++ using StormC4 can be found here: https://github.com/rosneru/AmiDiff

[alkis]

When you press the close button on the window, you actually close the window and then the (pMsg = (struct IntuiMessage *)GetMsg(pWin3->UserPort)) is accessing something that is not there.

[thyslo]

Hi alkis, thanks your answer!

This can indeed be a problem. So I've changed the code so that GetMsg() now hopefully is called with valid arguments. Sadly it still crashes. This is the changed event loop:

void Application::intuiEventLoop()
{
  struct Window* pWin1 = m_pLeftWin->IntuiWindow();
  struct Window* pWin2 = m_pRightWin->IntuiWindow();
  struct Window* pWin3 = m_pOpenFilesWin->IntuiWindow();

  struct Menu* pMenu = m_pMenu->IntuiMenu();

  while((pWin1 != NULL || pWin2 != NULL || pWin3 != NULL) &&
        m_bExitRequested == false)
  {
    // Waiting for messages from Intuition
    ULONG mask = 0;
    if(m_pLeftWin != NULL && m_pLeftWin->IntuiWindow() != NULL)
    {
      mask |= 1L << m_pLeftWin->IntuiWindow()->UserPort->mp_SigBit;
    }

    if(m_pRightWin != NULL && m_pRightWin->IntuiWindow() != NULL)
    {
      mask |= 1L << m_pRightWin->IntuiWindow()->UserPort->mp_SigBit;
    }

    if(m_pOpenFilesWin != NULL && m_pOpenFilesWin->IntuiWindow() != NULL)
    {
      mask |= 1L << m_pOpenFilesWin->IntuiWindow()->UserPort->mp_SigBit;
    }

    Wait(mask);

    struct IntuiMessage* pMsg;

    while ((pWin1 != NULL) && (pMsg = (struct IntuiMessage *)GetMsg(pWin1->UserPort)) ||
           (pWin2 != NULL) && (pMsg = (struct IntuiMessage *)GetMsg(pWin2->UserPort)) ||
           (pWin3 != NULL) && (pMsg = (struct IntuiMessage *)GetMsg(pWin3->UserPort)))
    {
      // Get all data we need from message
      ULONG msgClass = pMsg->Class;
      UWORD msgCode = pMsg->Code;
      APTR msgIAddress = pMsg->IAddress;
      struct Window* msgWindow = pMsg->IDCMPWindow;

      // When we're through with a message, reply
      ReplyMsg((struct Message *)pMsg);

      if(msgClass == IDCMP_MENUPICK)
      {
        //
        // Menupick messages are handled here
        //
        UWORD menuNumber = msgCode;
        struct MenuItem* pSelectedItem = ItemAddress(pMenu, menuNumber);

        if(pSelectedItem != NULL)
        {
          // Getting the user data from selected menu item
          APTR pUserData = GTMENUITEM_USERDATA(pSelectedItem);
          if(pUserData != NULL)
          {
            // Our menu user data always contain a pointer to a Command
            Command* pSelecedCommand = static_cast<Command*>(pUserData);

            // Execute this command
            pSelecedCommand->Execute();
          }
        }
      }
      else
      {
        //
        // All other messages are handled in the appropriate window
        //
        if(msgWindow == m_pLeftWin->IntuiWindow())
        {
          m_pLeftWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
        else if(msgWindow == m_pRightWin->IntuiWindow())
        {
          m_pRightWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
        else if(msgWindow == m_pOpenFilesWin->IntuiWindow())
        {
          m_pOpenFilesWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
      }

      pWin1 = m_pLeftWin->IntuiWindow();
      pWin2 = m_pRightWin->IntuiWindow();
      pWin3 = m_pOpenFilesWin->IntuiWindow();

      if(pWin1 == NULL && pWin2 == NULL && pWin3 == NULL)
      {
        // Exit because all windows are closed
        m_bExitRequested = true;
      }
    }
  }
}

With StormC debugger i can step trough the program. The crash occurs after the final return 0; from main().

Could it be that in my loop I forgot to reply a message? Could this lead to a crash?

[alkis]

I think you are still executing the inner loop.

Try adding a break to get out of the inner loop.

if(pWin1 == NULL && pWin2 == NULL && pWin3 == NULL)
      {
        // Exit because all windows are closed
        m_bExitRequested = true;
        break;      /* <---- add this */
      }

[Hewitson]

If you don't mind me saying, for such a simple program, it's quite unreadable. May I suggest using better indenting/formatting techniques? It will make your code much easier to debug.

[thyslo]

I've found a solution that seems to work. I've introduced two helper methods for generating the signal mask and for getting the next message from any of the opening windows. This also makes the code a little better readable.

Current event loop and the two helper methods:

void Application::intuiEventLoop()
{
  ULONG mask;
  struct IntuiMessage* pMsg;
  do
  {
    mask = signalMask();
    Wait(mask);
    while (pMsg = nextIntuiMessage())
    {
      // Get all data we need from message
      ULONG msgClass = pMsg->Class;
      UWORD msgCode = pMsg->Code;
      APTR msgIAddress = pMsg->IAddress;
      struct Window* msgWindow = pMsg->IDCMPWindow;

      // When we're through with a message, reply
      ReplyMsg((struct Message *)pMsg);

      if(msgClass == IDCMP_MENUPICK)
      {
        //
        // Menupick messages are handled here
        //
        UWORD menuNumber = msgCode;
        struct MenuItem* pSelectedItem = ItemAddress(m_pMenu->IntuiMenu(), menuNumber);

        if(pSelectedItem != NULL)
        {
          // Getting the user data from selected menu item
          APTR pUserData = GTMENUITEM_USERDATA(pSelectedItem);
          if(pUserData != NULL)
          {
            // Our menu user data always contain a pointer to a Command
            Command* pSelecedCommand = static_cast<Command*>(pUserData);

            // Execute this command
            pSelecedCommand->Execute();
          }
        }
      }
      else
      {
        //
        // All other messages are handled in the appropriate window
        //
        if(msgWindow == m_pLeftWin->IntuiWindow())
        {
          m_pLeftWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
        else if(msgWindow == m_pRightWin->IntuiWindow())
        {
          m_pRightWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
        else if(msgWindow == m_pOpenFilesWin->IntuiWindow())
        {
          m_pOpenFilesWin->HandleIdcmp(msgClass, msgCode, msgIAddress);
        }
      }

      if(!m_pLeftWin->IsOpen() && !m_pRightWin->IsOpen() &&
         !m_pOpenFilesWin->IsOpen())
      {
        // All windows are close: exit
        m_bExitRequested = true;
        break;
      }
    }
  }
  while(!m_bExitRequested);
}


ULONG Application::signalMask()
{
  ULONG signal = 0;

  if(m_pLeftWin->IsOpen())
  {
    signal |= 1L << m_pLeftWin->IntuiWindow()->UserPort->mp_SigBit;
  }

  if(m_pRightWin->IsOpen())
  {
    signal |= 1L << m_pRightWin->IntuiWindow()->UserPort->mp_SigBit;
  }

  if(m_pOpenFilesWin->IsOpen())
  {
    signal |= 1L << m_pOpenFilesWin->IntuiWindow()->UserPort->mp_SigBit;
  }

  return signal;
}


struct IntuiMessage* Application::nextIntuiMessage()
{
  struct IntuiMessage* pIntuiMessage = NULL;

  // Look for message in userport of left window
  if(m_pLeftWin->IsOpen())
  {
    pIntuiMessage = (struct IntuiMessage *)
      GetMsg(m_pLeftWin->IntuiWindow()->UserPort);

    if(pIntuiMessage != NULL)
    {
      return pIntuiMessage;
    }
  }

  // Look for message in userport of right window
  if(m_pRightWin->IsOpen())
  {
    pIntuiMessage = (struct IntuiMessage *)
      GetMsg(m_pRightWin->IntuiWindow()->UserPort);

    if(pIntuiMessage != NULL)
    {
      return pIntuiMessage;
    }
  }

    // Look for message in userport of open files window
  if(m_pOpenFilesWin->IsOpen())
  {
    pIntuiMessage = (struct IntuiMessage *)
      GetMsg(m_pOpenFilesWin->IntuiWindow()->UserPort);

    if(pIntuiMessage != NULL)
    {
      return pIntuiMessage;
    }
  }

  return NULL;
}

If you don't mind me saying, for such a simple program, it's quite unreadable. May I suggest using better indenting/formatting techniques? It will make your code much easier to debug.

I know what you mean. But I've a rule to myself to preferably not extend a line width of 72 chars. This leads to instructions/lines that go over two or more lines.

[thomas]

You should consider to share the same message port among all three windows. Would make your code much smaller and easier.

[thyslo]

Yeah I will look into this and maybe changing it. Yesterday I've found the example "CloseWindowSafely" in the autodocs which let me think about it.

[thyslo]

Implemented it now with a shared message port, and it works like a charm:-) And is by far more readable.

Thank you all for your help and hints!