SEModCommunity / SE-Community-Mod-API

Space Engineers Community Modding API
GNU Lesser General Public License v3.0
60 stars 47 forks source link

plugin update() method spam #86

Closed DraygoKorvan closed 10 years ago

DraygoKorvan commented 10 years ago

I'm getting a few reports that the motd plugin is causing an extreme lag situation, on investigation I am noticing that the update() method is being spammed on these servers extremely rapidly, causing a race condition within an older version of my plugin (I have since moved my mods execution code outside of update).

I think update should only be called once a second or maybe 4 times a second at most, but it seems to be getting called 100 times a second or more in some situations.

chessmaster42 commented 10 years ago

The interval for the plugin Update method is controlled by a timer value set in the Server.cs here: https://github.com/SEModCommunity/SE-Community-Mod-API/blob/master/SEServerExtender/Server.cs#L73

Right now it's set for a 200ms interval so there's no way it could be running faster than 5 times a second. Something else must be either causing your plugin to get loaded more than once into the manager, which shouldn't be possible (checks for unique GUID), or someone has another plugin that is messing with the server in some other way.

If you have bugs related to this on your Github repo can you link them here?

DraygoKorvan commented 10 years ago

https://github.com/DraygoKorvan/SEMotd/issues/3

supposedly but going over the log provided by him indicates otherwise. Plugin is only being loaded once, but update is being called rapidly at certain times leading to a race condition. Technically this should also impact your warp drive plugin.

This is all that was running in the update method: if (m_lastupdate + TimeSpan.FromSeconds(interval) < DateTime.UtcNow) { m_lastupdate = DateTime.UtcNow; if (enable) sendMotd(); }

Yet the log indicates that the mod is sending the Motd message 3 times. Indicates to me that SEServerExtender may not be waiting for the previous update thread to close allowing for more than one update thread to exist at the same time, allowing it to sendMotd() more than once. They indicate when this happens that SEServerExtender can become unresponsive. I don't know if they are running any other mods. I moved that function into my own thread, updating at a rate of once a second and the first guy indicates that the issue went away (Besides funkiness with the onplayerJoined thing).

I am assuming his server can get pretty loaded at times.

Tyrsis commented 10 years ago

Pretty sure update() is not the issue here but a bug in your plugin in the OnPlayerJoined that was causing the issue. This seems to be the case if you notice each message has a new thread (and you're spawning a new thread on each OnPlayerJoined call). If you check how Update is being called, he's firing it in an invoke, so there is no way Update is creating all those new threads unless Update is being hit by multiple instances. The console was being spammed, but you weren't seeing the message in game. (ie: a private message was being sent, not sendMotd() which is public).

I saw this happen on my server and had to comment out the OnPlayerJoined stuff completely. Once that was gone everything was fine.

chessmaster42 commented 10 years ago

The player events are being totally redone and there are new events for the character create/delete that was previously used as a stop-gap. Currently we don't have anything in place for player join/leave but we will soon.

As a workaround just have your plugin(s) check ServerNetworkManager.Instance.GetConnectedPlayers() periodically for changes.

DraygoKorvan commented 10 years ago

If you followed what I was saying in the other thread, I created a switch in the mod to turn off the messaging with the onPlayerJoined event because it was being rapid fired. The whole point of spawning the new thread in the onPlayerJoined was to release the thread that called onplayerjoined as quickly as possible. Removing the thread call made it worse. Every time you saw the message in the console it means that SEServerExtender called onplayerjoined.

chessmaster42 commented 10 years ago

With all of the recent changes around player data in the API I don't believe that this will be an issue anymore. I will close out once the release has gone live.