Zeex / samp-plugin-crashdetect

Crash/error reporting plugin for SA-MP server
http://forum.sa-mp.com/showthread.php?t=262796
BSD 2-Clause "Simplified" License
116 stars 23 forks source link

Control long_call_time options AND bug fix. #78

Closed Y-Less closed 4 years ago

Y-Less commented 4 years ago

Original PR:

This introduces script-side control of the long_call_time errors (#76), for example if you know a function is slow and just want to suppress the warning entirely. You can also reset the timing if, for example, you want to know which sub-call is slow in a function that may make lots of calls.

For example, I've just updated YSI:

https://github.com/pawn-lang/YSI-Includes/commit/011ca59b49c65f1d4ead38ca14f16aaa288a898e

During startup, the code generation is very slow, so this disables the warning for that case.

https://github.com/pawn-lang/YSI-Includes/commit/5d6ee138c0f2397fd66fe510d15a60545d82fb3a

During testing one test could be very slow while all the others are fast. This will reset the timing check between every test so that you get a warning for each slow test, not just one warning that all the tests together are slow.

You can also totally disable the long call checks now with:

long_call_time 0

New PR:

In the course of more testing, I found a major bug in the code as it was (even before these new features). Printing the stack trace modifies amx->stk and amx->frm, which is normally not a problem because the stack traces are printed when the code crashes. However, now I was printing traces on still running code. Because amx->stk et.al. aren't always updated from the local copies in amx_Exec, this would sometimes clobber useful bits of the stack.

Sadly, this has meant that I've had to switch to the originally suggested method (sort of) of polling from inside amx_Exec. To not slow things down too much it checks for a long time delay every 5000 instructions, not in a separate thread. This ensures that we can back up the important registers and pause execution while the stack trace is printing.

Zeex commented 4 years ago

Thanks 👍

I have a concern about storing long_call_time_next_ as a global: what if a public calls another public via CallLocal/RemoteFunction? Wouldn't that function overwrite long_call_time_next_ if it also modifies this option?

Zeex commented 4 years ago

Oh, never mind, I see that long_call_time_next_ is modified only during top-level public calls. It should be ok then.

rt-2 commented 4 years ago

Is there going to be a new stable release for this? Thank you

Zeex commented 4 years ago

@rt-2 here you go: https://github.com/Zeex/samp-plugin-crashdetect/releases/tag/v4.20

continue98 commented 4 years ago

it is good that such diagnostics have been added, but it causes false positives (for example, if you use nested loops (3 or more loops))

For example:

#if !defined VEH_RADIO_MAX_PLAYER_PLAYLISTS
    const VEH_RADIO_MAX_PLAYER_PLAYLISTS = 30;
#endif 

#if !defined VEH_RADIO_MAX_ITEM_IN_PLAYLIST
    const VEH_RADIO_MAX_ITEM_IN_PLAYLIST = 10;
#endif 

#if !defined VEH_RADIO_INVALID_OWNER_ID 
    const VEH_RADIO_INVALID_OWNER_ID = -1;
#endif 

static g_player_playlists_owner_id[MAX_PLAYERS][VEH_RADIO_MAX_PLAYER_PLAYLISTS][VEH_RADIO_MAX_ITEM_IN_PLAYLIST];
public OnGameModeInit()
{
    for (new i = 0; i < MAX_PLAYERS; i++)
    {
        for (new j = 0; j < VEH_RADIO_MAX_PLAYER_PLAYLISTS; j++)
        {
            for (new p = 0; p < VEH_RADIO_MAX_ITEM_IN_PLAYLIST; p++)
            {
                g_player_playlists_owner_id[i][j][p] = VEH_RADIO_INVALID_OWNER_ID;
            }
        }
    }
    return 1;
}

1000 30 10 iteartion.

@Y-Less, @Zeex

Y-Less commented 4 years ago

You can tweak the limits, but that doesn't sound like a false positive, that sounds like exactly the sort of case this is designed to detect. Deeply nested slow code.