Description/Reasoning for the current rules being enforced by default about rubocop-thread_safety HOT 4 CLOSED

The readme attempts to explain the reasoning ("unsynchronized mutation of state"). Instance variables in class methods aren't inherently un-safe.

The README touches briefly on the fact that mutating state without synchronizing it is unsafe, which should be obvious to anyone who understands threading, but the cops flag using them at all, which isn't explained to outsiders how something that is set-and-forget, read-only can be a threading danger. I'm not saying it's a bad thing to get people thinking about what they're doing more closely, but for people who don't know enough about threading practices, there could be more done to guide them on what they're doing wrong when they see an error pop up, IMO. If all they're using is boot time variables and never mutating the values, they would still see this error and end up confused as to what danger they're in instead of having more information on what that specific cop is targeting. The weight of a full mutex shouldn't be really needed for something that's boot-value only (like you said, hopefully safe)

It's definitely a complex problem, so that's why I think the docs could use some expansion from someone who knows what the target is. Some brief explanations on why thread.new is a default rule, instead of an optional add-on, etc

After getting some more details from you I think that's enough for me to put together a PR when I have time

from rubocop-thread_safety.

why is plain thread.new not safe, but thread.new with an injected value "safe"

That's a bug. They should be treated the same.

there are legitimate use cases for creating your own threads outside of your queuing system

Yup. I've created threads in my apps in couple different contexts where I need to disable this cop. Rake tasks and using threads to write specs for race conditions come to mind.

understand the reasoning behind items like "no ivar in class methods"

The readme attempts to explain the reasoning ("unsynchronized mutation of state"). Instance variables in class methods aren't inherently un-safe. The cop assumes the class is used in a multi-threaded application, where runtime mutations are un-safe.

I think you're pointing out the difference between run-time and boot-time mutation. That is a fundamental shortcoming of this project and static analysis of Ruby in general. Applications almost always mutate global state at startup, which is hopefully safe. This project cannot differentiate between start time and run time. Please disable the cops when you know they are pointing out problems that can't lead to threading bugs.

maybe I'm wrong but I don't see what thread issues would exist for class instance variables that wouldn't also exist for class variables

Class variables trigger a cop in base RuboCop, so we didn't add one here. You're right about their safety.

from rubocop-thread_safety.

Thanks for engaging! Documentation is definitely a shortcoming of this project.

from rubocop-thread_safety.

We can close this now I think. No reason just to leave it open just while I try to find some extra time

from rubocop-thread_safety.