Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch ThreadInterruptedException in Server.cs when closing stream/client in SendLoop #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

darrencperry
Copy link

These were getting through and logging as errors when client disconnects due to socket exceptions, trying to match functionality with ThreadAbortException.

It's possible this isn't the only scenario where a ThreadInterruptedException is thrown though? Maybe we do still want to log it?

@miwarnec
Copy link
Collaborator

@darrencperry is there an easy way to reproduce it?

@miwarnec
Copy link
Collaborator

and what do you mean with 'it still gets through to here when aborting'? - aborting what exactly?

@darrencperry
Copy link
Author

It's not easy to reproduce without a temperamental network.

That line you quoted is just copied from the above exception comment, I'm just trying to replicate the functionality of that error but I'm not 100% on whether a thread interrupted exception should be treated the same way as thread aborted exception.

I can't do much more on this in the near future as our project moved over to Ignorance, sorry!

@miwarnec
Copy link
Collaborator

miwarnec commented Feb 4, 2021

sorry, didn't have time for telepathy in a while.
can you confirm that the issue still exists in the latest version?
did a major rewrite in 2021 :)

@darrencperry
Copy link
Author

sorry, I'm not working with Telepathy or Mirror any more so I won't be able to do any more work on this, this pr solved an issue I was having at the time but feel free to close it, thanks!

@evandroabukamel
Copy link

Hey, guys. I'd like to revive this topic. In my game we are just updated and old version of Mirror to v93.0.1 and we still face this problem. The image bellow is the count of those logs in a separate production environment. It's not much but it happens in very specific case and we're testing only with ~6,000 players, we have around 100,000 in total, so this log tend to increase if we release this update globally.

Today I applied this change in our code and started to test in a separate environment. Let's wait.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants