You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
You'll notice that we change the state of struct after this await call. This is quite dangerous because our future can die at any point .await is called. This is quite tricky to manage and easy to overlook. In reality, what we want to do is to prevent this future (and others similar) to not be able to be dropped in the middle of anywhere in the future (ie: always complete future). The way this is normally done is via tokio::spawn, but this is a very heavy handed way to handle this.
What I believe would be a good solution is to make a future wrapper that holds the state of if the inner future completed. If it was not completed, implement a drop function that will move the future to a background spawn and ignore the results. This in theory should give us the best of both worlds. We'll be able to keep the future on the stack, but in the rare event the future gets dropped in the middle of a critical state managed future, it'll still execute but as a background spawn.
The text was updated successfully, but these errors were encountered:
Please review the implementation of DropGuard and consider how this behavioral change might impact any existing workflows.
cc: @allada, @MarcusSorealheis
If you look at the following code:
nativelink/nativelink-util/src/evicting_map.rs
Line 437 in d9922b3
You'll notice that we change the state of struct after this await call. This is quite dangerous because our future can die at any point
.await
is called. This is quite tricky to manage and easy to overlook. In reality, what we want to do is to prevent this future (and others similar) to not be able to be dropped in the middle of anywhere in the future (ie: always complete future). The way this is normally done is viatokio::spawn
, but this is a very heavy handed way to handle this.What I believe would be a good solution is to make a future wrapper that holds the state of if the inner future completed. If it was not completed, implement a drop function that will move the future to a background spawn and ignore the results. This in theory should give us the best of both worlds. We'll be able to keep the future on the stack, but in the rare event the future gets dropped in the middle of a critical state managed future, it'll still execute but as a background spawn.
The text was updated successfully, but these errors were encountered: