When Readonly isn’t Readonly in C#
Вставка
- Опубліковано 4 січ 2023
- Join the NDC Conferences Giveaway: mailchi.mp/nickchapsas/ndc
Check out my courses: dometrain.com
Become a Patreon and get source code access: / nickchapsas
Hello everybody I'm Nick and in this video I will explain what ReadOnly collections such as IReadOnlyList and IReadOnlyDictionary are and how they really work behind the scenes.
Workshops: bit.ly/nickworkshops
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: bit.ly/ChapsasGitHub
Follow me on Twitter: bit.ly/ChapsasTwitter
Connect on LinkedIn: bit.ly/ChapsasLinkedIn
Keep coding merch: keepcoding.shop
#csharp #dotnet
Personally, if a dev starts casting read only collections (from properties that are in our code base) to mutate them, they'll be met by a wall in code review 😋
Right up until the point it's the only way to fix some obscure bug
@@asedtf Well this would only be the case if the read-only property was from an external library, but here we are talking about read-only properties we put in our code.
It will also be met with a friendly visit from the clue bat
@@asedtf "Don't remove this comment, I don't know why but removing this comment causes the program to crash with no information, so please do not remove this comment"
If you catch it, that is
The moment you're casting a IReadonlyList to a List, you're fully aware that you're doing something hacky. So we're at the same level of hacky/forbidden stuff as using reflections to get the private member.
I think it is redundant to use immutable/frozen collections, because the consumer of those objects should respect how they are returned, even the simple cast to ReadOnlyList should be enough, if they cast it back and mutate the list, that's undefined behaviour and it's their fault if something breaks.
Plus if someone really wants to mutate the data, they can always bypass the ReadOnly getter with reflection.
the problem is "it is their fault something breaks" doesn't actually stop a subtle bug making their way into a program, costing lots of time and money to find and fix.
yes, it is their fault - but much of programing is making certain faults impossible - it is like accepting a non-nullable parameter and returning a non-nullable result, and saying "if anything here is null, then that is the fault of the thing that created the null" - agreed, but you want the code to make making null impossible, not blame the "incorrect" use elsewhere.
@@andytroo If someone else IS using the code incorrectly then you have no control over what bugs they might create. If you return a list as IReadOnly you are saying that your code will work without issues if you respect that iReadOnly interface. If someone deliberately break that by casting it back into a list, thats on them. Don't want bugs? Use the code the was it was meant to be. If you for some reason don't want to, then you take the responsibility of fixing bugs that arise from that.
Also, as I mentioned already. You can return everything in a readonly wrapper, or even return a copy of the data, just to be sure. But if someone is willing to mutate data, that was supposed to be treated as immutable, chances are that they would just bypass these restrictions with reflection anyway, as they might have a very specific usecase (But, again, in that case the responsibility falls on them, not you)
Anyone who upcasts the interface will be punished and an MR will be rejected
I had to watch this video twice, and the third time actually code this out to truly completely understand what's been talked about here as someone who never had the need or used readonly collections. Thanks! it was worth it.
4:30 i dont mind these types of stuff
the reason we dont want people to have access to .Add() so they dont break the thing without knowing
if they cast it on purpose then they know they are doing something other than what they are suppose to do
also if people want to they can access and change anything anywhere anyway
and if people wanna have access to my internals some how and built on top of it they should be able to easily
i would go further and say, types are something that helps the developers, ide compiler, and not needed on runtime
I do understand your arguments and differences between both types.
I don't agree with "always use .AsReadOnly()" since the consumer doesn't know for certain which is the type behind the interface. If he starts to cast the list and adds item he uses the API wrong. You can do the same with the AsReadOnly() or FrozenSet even though it requires reflection (but the frozen type wont change when the original type changes).
You could argue never use IEnumerable since it effectively is immutable but the consumer could also cast it back to it's original type and mutate it. It's the same "problem" here.
Hope you understand what I'm trying to say. Nice informative video though to show the differences.
Another distinction on whether IReadOnly or AsReadOnly should be used are the concurrency requirements. Both of these are not necessarily thread safe as the underlying collection can change and a consumer of the IReadOnly collection may be iterating over a mutating collection. Frozen and Immutable collections will provide a point in time snapshot to the consumer which is thread safe.
why is a snapshot particularly thread safe?
@@ahmedifhaam7266 An IReadOnly interface only exposes the read methods of the underlying collection. Say thread A has the reference to a List and exposes it to thread B using the .AsReadOnly() interface. Thread A can still mutate the list while thread B is iterating over the same list (but exposed a IReadOnlyList) which can cause concurrency problems.
Now let's say thread A has an ImmutableList and exposes it to thread B. If thread A "mutates" it, what's really happening is that thread A receives a new instance of the list, leaving the old one (which thread B has a reference to) as it was. If thread B iterates at the same time, it's really iterating on the old version which can never change, so no concurrency problems.
You do need to think about how fresh the data needs to be for thread B. In my experience, freshness doesn't matter as much as having a consistent view of the data at some recent point in time for a lot of use cases.
Haha, I was waiting for that last note regarding unsafe code. :)
No matter how neatly we design our libraries, with all manners of safeguards and read-only members, always remember that a sufficiently determined consumer can simply use memory manipulation to do things you specifically tried to prevent. But at that point, it's their own fault if they encounter unexpected behavior.
In some way shape or form, there is always some kind of way to hack around things. But the thing is that it becomes very obvious that somebody has or is trying to hack around things.
Doing this makes it OBVIOUS that it is going on which should raise some questions as to why ,instead of blindly merging things in. In my opinion, readonly collections are there to make it OBVIOUS the intent for current and future devs. Which is very important!! 🙂
Most of the time we use ImmutableArray because we need thread-safety on the reading level as well and the items does not change that much on runtime. When public users need to access the items, there is a IEnumerable that points to the ImmutableArray. Normal lists, sets or dictionaries are rarely used as a backing storage - due to heavy multithreading code. In addition we have special-made collections that are mutable, but are thread-safe, supports INotifyCollectionChanged with support for range operations - which a normal ObservableCollection can´t do. But its good to know, that we could use ReadOnlyCollection for preventing collections to be modified from the outside. Thanks for sharing!
frozen vs readonly - readonly declares the consumer can't/shouldn't change it, but frozen also promises the supplier won't change it - that extra guarantee can be nice, in some multithread circumstances
I might be wrong here, but I think that the main goal of this video is to be aware of how ReadOnlyCollections are implemented.
It's not about the probability of casting to the wrong type or the possibility of leaking in a production codebase.
Bad things happen.
It's about all of the above
How will a leak happen actually of casting a readonly collection if it's our own codebase.
Great content, but I would explain a bit more why, someone as developer, would readonly collection exposed to "users". I think it would be very helpful to explain the benefits of "users" which aren't able to modify collections which the shouldn't. For stability and consistency. Many starting programmers don't know that. They are probably not aware that other developers might use their code/library in a wrong way.
I think it would be a great topic for you to handle to explain why...
Yeah, I was well aware of those caveats with IReadOnlyCollection.
I use it for 3 reasons:
1. I don't want to allocate full copies on the heap, and would rather return a collection that isn't safe to modify in an outer scope
2. I want to signal to myself and other developers that this is the wrong place to change something
3. There is no ReadOnly wrapper for HashSet
As for #2, we legit had places in our code where changes would just vanish into the aether if we did a "Add()" to the returned collection.
To protect both myself and other devs from that trap in the future, I changed those functions to return IReadOnlyCollections.
IIRC we used to take the backing list and have an ICollection return type on the GetList and loop through the list with a yield for every element. This prevented access to the backing list.
Yield means ienumerable. Not optimal if you need to return a list or a set
@@zariumsheridan3488 Sure that's true. But, how much does a readonly list and set give you over IEnumerable? There are linq extensions to it as well.
@@pilotboba a lot, when you need O(1) instead of O(n) access by index or key.
@@zariumsheridan3488 For sure yes. Of course, your wrap class can also provide those methods as well. The whole point here is to prevent a client object from mutating the list. Of course, now in .Net 7/8 we will have many more native options to deal with this.
I appreciate the info and I enjoy your videos, Nick! In my experience you need to assume the people you're working with aren't complete idiots and that your code review process will catch things. My last job was full of devs that were fanatical about locking down code as much as possible and it was a nightmare when clients changed their minds on something.
Activating the notifications bell for Nick Chapsas is the best decision I made for my career ❤
I think it would have been worth showing foreach benchmarks. When using readonly interfaces or readonly collections, the struct enumerator gets boxed when GetEnumerator is called. This causes an allocation everytime you foreach over the collection, which I found surprising and disappointing.
If I understand correctly, the only real benefit of frozen collections is they don't change when the original data changes which is useful for creating snapshots of the data as opposed to simply making it immutable.
And they perform better than any other data structure on read operations
@@nickchapsas how is it more performant, just curious.
Frozen collections can be attacked with Reflection too right?
Sure but it's way trickier because they are backed by Immutable data structs. To mutate readonly all you need is a reference to the backing data struct which can be retrieved in multiple ways
@@nickchapsas Ah yes, arms race continues!
Nothing a little memory editing magic can't fix...
When you start using reflection, you give up all right to live in a predictable, ordered world where the compiler can make guarantees.
Starting to worry about bad programmer that want to "hack" the API is a rabbit hole. Using AsReadOnly only make it more of a chore but you can still "hack" it by accession the private field with the reflection. The quantity on code require to "hack" the API is irrelevant since you can always put that code in a function and then do it in a one-liner.
Maybe there is situation where doing your maximum to protect you API is require but in day-to-day case I would not worry about it. If the other programmer want to do shit code he will do shit code whatever you do.
how can you cast and add to a API you are consuming, there is an interface in between right.
No wonder I have trust issues, even C# is lying to me...
I like to use both Read-only and Immutable collections when I want to make clear the intent of the collection. What I have stopped using are the interfaces that represent a collections as they are often the cause of the mess
i am looking for internship
Don't say "it just casts it", when it's quite the opposite.
Well, as demonstrated/told by Eric Lippert even readonly modifiers are a "lie" (•_•) They should've called them... ( •_•) "readonlies" (⌐■_■)
The whole collection/interface hierarchy seems to have become quite jumbled -- Immutables that are not immutables, readonlys that are not readonly, frozen that can be made into not immutable immutables etc. etc. ("Kinds of Immutability" by Eric is a good read). Unavoidable when you grow a framework piecemeal, but I do wish they were a little more intuitive . C'est la vie :)
Lifegoal achieved:
Nick calls my community contribution to dotnet "super super helpful" :) (dictionary.AsReadOnly as extension method)