client/New() trigger failure? in Bug Reports

ID:105144
 
Nov 29 2010, 9:11 pm
Not a bug
BYOND Version:479
Operating System:Windows XP Pro
Web Browser:Firefox 3.6.12
Applies to:DM Language
Status: Not a bug
This is not a bug. It may be an incorrect use of syntax or a limitation in the software. For further discussion on the matter, please consult the BYOND Developer forums.
Descriptive Problem Summary:
In very rare, nearly impossible to replicate circumstances, it appears client/New() in some unknown unique case, fails to fire.

Numbered Steps to Reproduce Problem:
Can not reproduce.

Code Snippet (if applicable) to Reproduce Problem:
proc/UpdateUserlist()
    //Update allusers list (sorting)
    var/list/allusers=list()
    for(var/mob/LM) if(isnull(LM.icon_state)) allusers += LM
    for(var/mob/LM) if(LM.icon_state=="away") allusers += LM
    for(var/mob/LM)
        if(LM.client)
            winset(LM,null,"userlist.cells=1x[clients];cusers.text=\"Users Online: [clients]\"")
            var/ultracker=0
            for(var/mob/ALU in allusers)
                if(ALU.client)
                    ultracker++
                    LM << output(ALU,"userlist:1,[ultracker]")

var
    clients=0
    maxclients=0

client/New()
    clients++
    if(world.cpu > 94)
        src << "<font color=#f00>* ERROR Server too busy please try again in a few minutes</font>"
        return
    if(byond_version < world.byond_version)
        src << "<font color=#f00>* ERROR Please update your BYOND client (You: <b>[byond_version]</b> & Server: <b>[world.byond_version]</b>)</font>"
        return
    if(clients > scfg["max_users"])
        src << "<font color=#f00>* ERROR Server is full</font>"
        return
    if(key in throttled)
        src << "<font color=#f00>* ERROR - Connection Throttled - Please wait 60 seconds</font>"
        return
    if(ckey in banned_keys)
        src << "<font color=#f00>* ERROR - Connection Refused - Key is banned</font>"
        return
    if(address in banned_ips)
        src << "<font color=#f00>* ERROR - Connection Refused - IP Address is banned</font>"
        return
    if(computer_id in banned_pcs)
        src << "<font color=#f00>* ERROR - Connection Refused - Computer ID is banned</font>"
        return
    for(var/mob/M)
        if(M.client && M.client.computer_id == computer_id)
            src << "<font color=#f00>* ERROR Detected a connection from your computer from another connected user</font>"
            return
    if(copytext(lowertext(key),1,7) == "guest-")
        if(scfg["allow_guest_logins"]==1)
            src << "- You are not logged in to BYOND, features will be restricted, and any options will not be saved"
            var/mob/nwmob = new()
            nwmob.name=key
            nwmob.isguest=1
            nwmob.key=key
        else
            src << "<font color=#f00>* ERROR This server does not allow guest logins</font>"
            return
    else
        if(fexists("saves/users/[ckey].sav"))
            var/mob/nwmob = new()
            LoadUsrSave(nwmob,ckey)
            nwmob.name=key
            nwmob.gender=gender
            nwmob.key=key
    return ..()

client/Del()
    clients--
    return ..()

mob/Login()
    cnct_ts = world.time
    if(scfg["log_connections"]==1) WLog("connection","[key] logged in from ip [client.address]")
    if(!(key in throttled)) throttled += key
    if(clients > maxclients)
        maxclients = clients
        SaveMaxClients()
        StatusMsg("info","<b>New Record!</b> Max clients connected is now [maxclients]")
    ChatMsg("join","<b>--\> ([key])</b> has joined [world.name]",src)
    loc=locate(1,1,1)
    UpdateUserlist()
    InitInterface()
    spawn() AwayLoop()
    spawn() FloodLoop()
    StatusMsg1("info","Welcome [key]. Your host is [world.internet_address], running [world.name] version [GAME_VER], on BYOND version [world.byond_version].<br>This server has been running for [Duration(world.time)].")

mob/Logout()
    if(scfg["log_connections"]==1) WLog("connection","[key] logged out")
    SaveUsrSave()
    ChatMsg("quit","<b>\<-- ([key])</b> has quit [world.name]",src)
    UpdateUserlist()
    del(src)


Expected Results:
Correct clients var which reflects current login count.

Actual Results:
clients var is too low indicating client/New() is sometimes failing.

Does the problem occur:
Every time? Or how often? Rarely
In other games? Untested
In other user accounts? Most likely
On other computers? Probably

When does the problem NOT occur?
Generally this is not a problem.

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? (Visit http://www.byond.com/download/build to download old versions for testing.)
Untested.

Workarounds:
We could use the allusers list var in the updater as the for loop always seems to flect the accurate count. The var tracker is the issue.

Other Notes:
I absolutely hate reporting things I can't at least reproduce to a degree. I suspect this is maybe a packet issue causing client/New() to not fire but I can't prove anything.

I have tried to reproduce locally using guest account feature and autoit aid slamming 2 connections per second but it is always right on the money. This is a really frustrating issue.
#1 Nov 30 2010, 9:26 am
If this can't be reproduced, then reporting it as a bug is premature. A bug that can't be reproduced can't be investigated, either. But at this point I still wouldn't rule out a code problem.
#2 Nov 30 2010, 11:37 am
I suppose I'll just leave well enough alone. It is logically impossible that a globally defined var that begins at zero become desynced based on the manner I posted. Those proc's are the ONLY place where that var is adjusted. Anyhow, it's my fault apparently.
#3 Nov 30 2010, 11:59 am
In theory you're right, if client/New() and client/Del() are always supposed to be called in the expected way, the var should never go negative. I'd have to verify that was actually the case with how the procs are called, but even so the problem stands that you can't reproduce it. If you can't, I can't. And the reason I think it's better off closed than Unverified is that I'm not sure code problems can entirely be ruled out yet. That doesn't mean I'm saying the issue is your fault--it means I don't think due diligence has been done in investigating it yet, and reporting a bug that can't be reproduced is always way premature. Even with intermittent bugs users have been able to provide a set of steps to follow to hopefully trigger the problem; this doesn't have that.

I'm not sure it matters terribly either way anyway. A foolproof workaround is to simply count your active clients with assigned mobs on an as-needed basis.
#4 Nov 30 2010, 12:37 pm
Lummox I have no problem sending the full source if you'd host for a few days ... the issue is rare. At first it was at -1 for the offset but as it stands currently the offset is -3. I know the issue exists but the fact I can't replicate it annoys me. Sometimes users have to relog in order for the chat to work for them. I really can't say what the issue could be, but I have shown the source to some of the more veteran DM programmers and they are equally baffled.
#5 Nov 30 2010, 1:17 pm
You can send me source at LummoxJR@aol.com if you like; however I'm in no position to host the project for an extended period. What would be much more helpful would be if you gathered logging information on any changes to the count. Right now there's no information at all, which makes the bug impossible to investigate.
#6 Nov 30 2010, 3:03 pm
Ok Lummox I'll send you the source. In terms of logging information the first offset was seen within 24 hours of hosting and the other two was pointed out to me. It's a really weird bug but I am taking your advice and implementing a for loop counter strategically in hopes to combat the desyncing.

On a side note, any programming criticism is warmly welcomed.
#7 Dec 2 2010, 7:34 am
You could always add some code that specifically meant just for monitoring this situation. Keep a log file of every client connection/termination, and in the same line in the file that records the action, have it also display the new (possibly old and new) count.

You could then also create a monitor function with a loop that just checks in on the count every minute (or every 30 seconds or even less). The loop could calculate itself what the value actually should be and what it is. If they are equal, you can optionally output an "all ok" message, or do nothing; if they are not equal, report it in the log.

The finer grain you make the monitor loop the more precisely you can determine when it happened and then possibly notice some patterns.
#8 Dec 2 2010, 10:44 am
In particular, at each New() or Del() event for clients it would be helpful to output the client's ID (via \ref[src]), the key, the IP address, and the computer_id. Some connections won't have keys or computer_ids, like world-to-world messages. The ID will help determine if Del() has been called without a corresponding New() and if so in what circumstances.