Sync Rc: what's the worst that could happen?


#1

(answer: nasal demons of undefined behavior)

This necessarily is a question that depends on implementation details in the stdlib, since breaking the type system like this means unsafe data races and full on specified undefined behavior.

Pest’s meta parse tree is defined using Rc, since pest’s compiler does all of its work in a single thread. In pest-ast, however, we’d benefit from stuffing a meta parse tree into a global so that we can reuse that computation between proc-macro calls.

The stuffed data structure can be considered “leaked”, as none of the Rcs will be dropped in the future, and we rely on the OS to clean up. Simplifying things a bit, there is no interior/shared mutability within the structure, but working with it necessarily means cloning Rcs.

If Rcs are cloned and dropped from multiple threads, what’s the “worst” that can happen? Reference counts being higher than accurate is safe and even fine as the data is “leaked” already, but reference counts being lower than accurate would lead to freeing while Rcs still exist.

I’m thinking the only potentially safe way to do this is to have a lock that’s required to be locked when cloning or dropping an Rc (thus writing to the ref count). A lock shouldn’t be necessary for operations that just read the value and/or ref count. But I want to check that this is sound before going forward with it.

(That, and pest 3.0 may end up changing the story a lot.)


#2

It seems like you have your answer in the first line of your post.


#3

The difference between Rc and Arc is synchronization when changing reference count. If count gets out of sync between threads you may have not just a memory leak, but also double free, and use after free, which are much nastier.

Mutex<Rc<T>> would be safe if there was a guarantee that every use of the refcount is only through that Mutex. The Mutex would ensure no other thread is modifying the refcount at the same time.

However, because Rc can be cloned, it could be cloned out of the Mutex and incremented/decremented independently, and that’s asking for trouble.


#4

This might be relevant to your interests.


#5

Why not just leak it for real with Rc::into_raw?

If two threads race trying to increment the count, then the likely outcome is that one of their effects will be missed, so the count will be low. That is, they may both read the same starting count, add their 1, then both write back the same count+1, where it ought to end up at count+2.

Similarly for a racing decrement, we could end up with count-1 when it should be count-2. For racing increment and decrement, it could go either way, and throw in more racing threads for worse results.


#6

Oh, I suspect the answer will be that these Rc are internal to the parse tree.

Another option to all this is to store the tree in a thread-local instead of a full global, and then you don’t need Sync at all.


#7

If you’re looking for a workaround with shoddy security, here it is:

I had to use unsafe and lie about Send, because ThreadLocal doesn’t have a choice on which thread the destructors run. The global-ish thread_local!{} doesn’t have that problem.


#8

Thanks everyone!

I think I’m going to wait for pest:3.0 to cook a bit more before doing things along these lines, because it might reduce the need to have pest-ast run in multiple proc-macros at all.

My case is complicated by the fact that working with the tree basically requires cloning the Rc a lot, so the only workable Sync solution would be to ThreadLocal it or transmute-hack it to “atomicize” the Rc.

In any case, I’m pretty far off of having the functionality to actually need this hack, but I understand the implications a lot better now!