ErikMinekus / sm-ripext

SourceMod REST in Pawn Extension
https://forums.alliedmods.net/showthread.php?t=298024
GNU General Public License v3.0
136 stars 38 forks source link

Server crash #7

Closed CrazyHackGUT closed 4 years ago

CrazyHackGUT commented 7 years ago

https://crash.limetech.org/eqo3npik5kk2

Game: TF2 Extension version: 1.0.1

ErikMinekus commented 7 years ago

Do you have the SourcePawn code that caused the crash?

CrazyHackGUT commented 7 years ago

Yes. As it seems to me, the crash is due to this plugin: https://github.com/CrazyHackGUT/Discord/blob/master/addons/sourcemod/scripting/discord_sb.sp It uses the API of this plugin: https://github.com/CrazyHackGUT/Discord/blob/master/addons/sourcemod/scripting/discord.sp He collects the request here: https://github.com/CrazyHackGUT/Discord/blob/master/addons/sourcemod/scripting/discord/UTIL.sp#L34-L145

kinsi55 commented 7 years ago

Just had my server segfault because of ripext as well, CS:GO on Linux for me. Unfortunately, i dont have anything that would be useful do debug it, no accelerator, and no idea what could've caused it. The way i use it is that i initialize a global HTTPClient which then executes multiple (only get) requests all the time, some of them do parsing, others are simply "fired and forgot" (empty callback).

How i know it was ripext causing it?

srcds_linux[12622]: segfault at 0 ip 00000000ea989fb4 sp 00000000ff9ab7e0 error 4 in rip.ext.so[ea977000+b3000]

asherkin commented 7 years ago

That is indeed the same crash, they're both crashing around here: https://github.com/ErikMinekus/sm-ripext/blob/edc2f4c91186907b05a2f03fb6ab37540ff08592/extension.cpp#L107-L115

Either callback or forward is bad (looks like the former, and more likely to be that one, but I can't be 100% without debug symbols).

EDIT: Actually, it is definitely callback that is bad.

EDIT 2: Looking at the OPs crash dump, ebx (which I believe to be &callback) is 0x00000000`, so this is probably a logic bug with the queue handling somewhere.

EDIT 3: Looking again, I was wrong above - forward is the NULL one here, Deathknife on Discord pointed out that the IChangeableForwards created by RIP are never freed, and it doesn't look like there is any error checking around forward creation - I imagine that something is probably getting exhausted in SM here and CreateForwardEx is returning NULL.

EDIT 4: Hmm, no, that would cause the crash to happen at HTTPClient::Request... I'm bored of looking at ASM now.

ErikMinekus commented 7 years ago

I've added a null check in HTTPClient::Request and ReleaseForward calls to 1.0.3. If that doesn't help I'll look into properly terminating threads when plugins are unloaded, instead of abusing the forward system.

CrazyHackGUT commented 7 years ago

Two new crashes :( https://crash.limetech.org/rek3l73fgny2 https://crash.limetech.org/mdjtmfvqacos

Kentusha commented 7 years ago

Extension crash CSGO server

CrazyHackGUT commented 7 years ago

Any update? :< This is great extension. But crashes...

CrazyHackGUT commented 7 years ago

I fixed this problem in my own fork. Maybe i publish him on AlliedModders during this week.

awkspace commented 7 years ago

@CrazyHackGUT Why not make a pull request?

CrazyHackGUT commented 7 years ago

@awkspace I think, developer forget about RiP. You can download my fixed version from here.

peace-maker commented 6 years ago

Isn't std::queue.pop() destroying the front object?

https://github.com/ErikMinekus/sm-ripext/blob/8b421f292e6cfa6de20fa35878f9502df1e2072a/extension.cpp#L107-L108

@ErikMinekus So you're getting a reference to that object and destroy it right away afterwards. Maybe move the pop() call after you copied the struct values? I can't reproduce the crash, so can't test it.

ErikMinekus commented 6 years ago

Hey @peace-maker,

pop() does call the object's destructor. Thanks, I'll try moving it down.

It is indeed difficult to reproduce, it seems to happen at random after running the extension for a while. I figured since the callback is constructed in a separate thread and then passed to the main thread, it was sometimes GCed before it could be processed. But maybe this is the issue instead.

asherkin commented 6 years ago

it was sometimes GCed before it could be processed.

There is no garbage collector.