Timer.device memory leak?

[leemmcc]

Hi everyone,

I'm trying to write a simple piece of code to use timer.device to run a task every second.

It seems to work ok (I get Signal recieved in the shell window) but when I ctrl+c to end the program it locks, resets and gurus. Clearly there's a leak somewhere but I can't track it down.

#include <devices/timer.h>
#include <proto/dos.h>
#include <proto/exec.h>

#define TIME_SECONDS 1
#define TIME_MICROSECONDS 0

BOOL timerDeviceOpen = FALSE;
LONG timerSig;
BOOL timerRunning = FALSE;
struct timerequest *timeReq;
struct MsgPort *timerPort;

/**
 * @brief Dealloc and cleanup timer
 */
void clearTimer()
{
    if (timerDeviceOpen == TRUE){ Printf("Close timer device\n"); CloseDevice(&timeReq->tr_node); }
    if (timeReq != NULL){ Printf("Delete timerrequest\n"); DeleteExtIO((struct IORequest*)(timeReq)); }
    if (timerPort != NULL){ Printf("Delete message port\n"); DeleteMsgPort(timerPort); }
}

/**
 * @brief Start next tick of timer
 */
void tickTimer(){
	timeReq->tr_node.io_Command = TR_ADDREQUEST;
	timeReq->tr_time.tv_secs = TIME_SECONDS;
	timeReq->tr_time.tv_micro = TIME_MICROSECONDS;
	SendIO(&timeReq->tr_node);
}

int main()
{
	timerPort = CreateMsgPort();

	if (timerPort)
	{
		timeReq = (struct timerequest*)(CreateExtIO(timerPort, sizeof(struct timerequest)));

		if (OpenDevice("timer.device", UNIT_VBLANK, &timeReq->tr_node, 0) == 0)
		{
			Printf("Starting!\n");
			
			timerDeviceOpen = TRUE;

			tickTimer();

			timerRunning = TRUE;

			while (timerRunning) 
			{
				/**
				 * Wait for a signal from either ctrl+c or the timer
				 */
				ULONG sig = Wait(SIGBREAKF_CTRL_C | 1L<<timerPort->mp_SigBit);

				/**
				 * Signal from the timer
				 */
				if (timerPort && GetMsg(timerPort))
                {
					Printf ("Signal recieved\n");

					tickTimer();
                }

				/**
				 * Quit the app
				 */
				if (sig & SIGBREAKF_CTRL_C)
				{
					Printf("Ctrl-C received; timer stopping.\n");
                    timerRunning = FALSE;
				}
			}
		}
		else
		{
			Printf("Could not open timer.device!\n");
		}
		
	} 
	else 
	{
		Printf("Could not create Msg Port!\n");
	}

	/**
	 * Dealloc and clean
	 */
	clearTimer();
	Printf("Done!\n");

	return 0;
}

[meynaf]

A memory leak wouldn't crash, it would just not free all memory.
Here you have called SendIO but what you've sent isn't cancelled upon exit. Perhaps some call to AbortIO is needed.

[Toni Wilen]

Pending timer requests must be canceled first. For example AbortIO() + WaitIO() does it.

[Thomas Richter]

GetMsg() is not exactly the right way to handle IO request. You send out the request with SendIO(). You test whether the request is done with CheckIO(), which returns the request when done, or NULL if it is not. CheckIO() does *not* remove the request. Use WaitIO() for that. It waits for the request, then returns when its done, and removes the request from the port, ready to re-use it. WaitIO() does of course not block if the request is already done. To abort a request, use AbortIO(), then WaitIO().

[leemmcc]

OK, that makes sense. So basically because I'm not using a combination of WaitIO(), AbortIO() and CheckIO() I have no way of telling whether the request is fully complete before I starting a new request. And, in addition to that, I'm not checking the state of the request before exit.

Thanks for your help everyone.

[thomas]

You do it like this:

			ULONG timersig = 1L<<timerPort->mp_SigBit;
			ULONG sig = Wait(SIGBREAKF_CTRL_C | timersig);

			/**
			 * Signal from the timer
			 */
			if (sig & timersig)
			{
				Printf ("Signal recieved\n");
				WaitIO (timeReq);
				tickTimer();
			}

			/**
			 * Quit the app
			 */
			if (sig & SIGBREAKF_CTRL_C)
			{
				Printf("Ctrl-C received; timer stopping.\n");
				timerRunning = FALSE;
			}

At the end, just after the while(){} loop you call AbortIO and WaitIO.

Be warned that you must not call AbortIO if the timerequest was never used before. As you call tickTimer just before the loop, put the AbortIO immediately after the loop. Do not move it before or into clearTimer, although this looks like a logical place. (You can safely call AbortIO/WaitIO if the request already ended. No need to call CheckIO.)

[tygre]

Hi Leemmcc and all!

For AmigaMapPing, I also had to play with timer so I created a simple implementation with three functions:

• struct timerequest *timer_create(void); to initialise the timer.
• int timer_start(struct timerequest *timereq, int minutes); that starts a timer for minutes.
• BOOL timer_done(struct timerequest *timereq); to check if any signal received from the timer is for our timerequest.
• void timer_free (struct timerequest *timereq); to free/close everything once done with the timer.

It should be reusable "as is" in any project. You can find it here: its API and its implementation and an example. Hope that it can help!

Cheers!