[CC 1.6] 'repeat' program never clears message history

Version:
ComputerCraft 1.6 for Minecraft 1.6.4 (bug is within a lua program and unrelated to CC's internal code)

Description:
In the default CC 1.6 'repeat' program (non-beta version), the elseif-branch at line 73 is never executed, because the check for whether the event is a timer event is an else-branch of the rednet channel check, rather than an else-branch of the event type check. This leads to the program never clearing the message cache and eventually filling up the computer's memory (actual effects not yet tested in gameplay).
This is easily fixed by moving one of the "end" statements above the elseif-line checking for timer events and adjusting the indentation.

Here is the code segment in question: (EDIT: Updated code segment to show control flow problems better, thanks Bomb Bloke!)

if sEvent == "modem_message" then
    if nChannel == rednet.CHANNEL_REPEAT then
	    ... Packet fowarding code ...
    elseif sEvent == "timer" then
        ... *** This branch never executes! *** ...
    end
end

Steps to reproduce:
Let a computer run the 'repeat' program and transmit a large number of messages over a longer timespan.

Read the code again, they get cleared after a 30 second delay

Dan, you may need to take another look at this one. Let me chop down the quoted section a bit further:

if sEvent == "modem_message" then
	if nChannel == rednet.CHANNEL_REPEAT then
		...
	elseif sEvent == "timer" then
		**** Under what circumstance will this execute?
	end
end

Bomb Bloke, on 30 March 2014 - 07:07 AM said:

Dan, you may need to take another look at this one. Let me chop down the quoted section a bit further:

OP posted a simplified version of the logic, see full logic

pcall( function()
	local tReceivedMessages = {}
	local tReceivedMessageTimeouts = {}
	local nTransmittedMessages = 0

	while true do
		local sEvent, sModem, nChannel, nReplyChannel, tMessage = os.pullEventRaw()
		if sEvent == "modem_message" then
			--# Got a modem message, rebroadcast it if it's a rednet thing
			if nChannel == rednet.CHANNEL_REPEAT then
				if type( tMessage ) == "table" and tMessage.nMessageID and tMessage.nRecipient then
					if not tReceivedMessages[ tMessage.nMessageID ] then
						--# Ensure we only repeat a message once
						tReceivedMessages[ tMessage.nMessageID ] = true
						tReceivedMessageTimeouts[ os.startTimer( 30 ) ] = tMessage.nMessageID

						--# Send on all other open modems, to the target and to other repeaters
						for n=1,#tModems do
							local sOtherModem = tModems[n]
							peripheral.call( sOtherModem, "transmit", rednet.CHANNEL_REPEAT, nReplyChannel, tMessage )
							peripheral.call( sOtherModem, "transmit", tMessage.nRecipient, nReplyChannel, tMessage )
						end

						--# Log the event
						nTransmittedMessages = nTransmittedMessages + 1
						local x,y = term.getCursorPos()
						term.setCursorPos( 1, y - 1 )
						term.clearLine()
						if nTransmittedMessages == 1 then
							print( nTransmittedMessages .. " message repeated." )
						else
							print( nTransmittedMessages .. " messages repeated." )
						end
					end
				end

			elseif sEvent == "timer" then
				--# Got a timer event, use it to clear the message history
				local nTimer = sModem
				local nMessage = tReceivedMessageTimeout[ nTimer ]
				if nMessage then
					tReceivedMessageTimeout[ nTimer ] = nil
					tReceivedMessages[ nMessage ] = nil
				end

			end
		end
	end
end )

The forum's code boxes don't always make this sort of thing easy to see, so here's the code in Sublime Text with a bit of the code folded (lines 12-34 of this function are hidden, but no indentation is changed, and Sublime Text will only fold whole blocks). For clarity, OP's point is that the event handling for timers is inside the event handling for modem messages and will therefore never be reached.

On that note, I noticed this structure in the rednet API yesterday (line ~190):

    while true do
        local event, p1, p2, p3 = os.pullEvent()
        if event == "rednet_message" then
            ...
        else
            -- Got a timer event, check it's the end of our timeout
            if p1 == timer then
                break
            end
        end
    end

I sorta assumed it was intentional, given that it's an understatement to say that the odds of it messing up due to a stray "key" event are rather low (and it's a half-second timer to boot), but it may be you're inclined to change it. I dunno.

My bad, I shouldn't have left so much irrelevant code intact. It's pretty difficult to highlight relevant lines of code without formatting within code tags, and on top of that, the single quotes somehow got syntax-highlighted as comment markers.
I updated the original post with a code snippet that shows the problem better, thanks Bomb Bloke!

Just reporting that this problem still exists in 1.6.1 version of Computercraft.

Thanks. I've fixed this now, it will be in the next version of the mod