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

WeakRef requires rescuing RefError to avoid race condition #3

Open
daveola opened this issue Sep 15, 2021 · 2 comments
Open

WeakRef requires rescuing RefError to avoid race condition #3

daveola opened this issue Sep 15, 2021 · 2 comments

Comments

@daveola
Copy link

daveola commented Sep 15, 2021

There's an race condition in the implied usage of WeakRef.

The API only has weakref_alive?, and then delegated access to the referenced object. But the delegated access to the object cannot be protected by weakref_alive? since GC may occur between the check and the usage.

This means we basically always have to check for RefError, which basically makes weakref_alive? useless if we want to actually potentially use the object.

WeakMap usage is discouraged, leaving us with needing to add this functionality to WeakRef, which may break if WeakRef implementation changes (i.e., there is no good solution for this).

This is discussed at length here:

https://stackoverflow.com/questions/69185508/ruby-weakref-has-implicit-race-condition

I would recommend an addition to the API that will safely return a (non-weak) object if it's alive, or else nil, and obviously it's up to the user to realize that this will stop GC from happening on that object while they hold it.

@casperisfine
Copy link

It's not documented but that method already exist as weakref.__getobj__. I agree that it should have a public alias though.

The problem is that since WeakRef delegates everything, any alias has a small chance to break existing code.

@daveola
Copy link
Author

daveola commented Jan 26, 2022

While I agree that there is the undoc'd feature getobj, there are two problems:

  1. Using undoc'd code not in the API is likely to be broken in the future when something changes.
  2. I'm still pretty sure the race condition exists. There's the check on the existence of delegate_sd_obj, then the return of that object. In between those steps there could be a GC, and you'd be in trouble.

I actually mention the usage of getobj still having this problem in my original post to stackoverflow and my solution actually uses getobj, but getobj is not sufficient on it's own.

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

No branches or pull requests

2 participants