async-executor icon indicating copy to clipboard operation
async-executor copied to clipboard

1.9.0 may need to be yanked

Open james7132 opened this issue 1 year ago • 10 comments

https://github.com/bevyengine/bevy/issues/12044 reports that the update to 1.9.0 is SemVer compatible with the prior 1.7.x and 1.8.x releases, and is currently causing all new fresh fetches and builds of Bevy to lock up on start. Likely due to a bug with #93. Still trying to root cause the issue.

james7132 avatar Feb 22 '24 09:02 james7132

Thanks. I yanked 1.9.0 now.

taiki-e avatar Feb 22 '24 09:02 taiki-e

Thank you. I'll try to figure out why this was failing, and incorporate the findings and fixes into #102, as it does seem to fix one of the problems introduced.

james7132 avatar Feb 22 '24 09:02 james7132

FWIW, this version also breaks zbus.

zeenix avatar Feb 22 '24 09:02 zeenix

The problem seems to be coming from the use of thread-local AtomicWakers. Upon removing them, it seems to allow for the executor to properly work, albeit it's yielding to the OS for way too long.

james7132 avatar Feb 22 '24 10:02 james7132

I've narrowed the problem down to the fact that the executor was storing wakers in two separate locations: the Sleepers and the thread-local AtomicWakers, if a thread was awoken by the thread-local wakers, the sleeping state of the corresponding ticker was not updated.

These two different storage mechanisms for wakers likely needs to be consolidated and synchronized in some form.

james7132 avatar Feb 22 '24 10:02 james7132

@zeenix can you check if #102 at least fixes the deadlock? This took a quick glance at zbus and it isn't using Executor::run in any way, so this localizes the problem to just how ticking works.

james7132 avatar Feb 23 '24 02:02 james7132

Maybe my instinct here was correct, as we need to keep track of the sleeper state in one central location.

notgull avatar Feb 23 '24 05:02 notgull

I got Bevy to work without deadlocking. We were removing the waker from Sleepers but not from the LocalQueue when dropping Ticker.

james7132 avatar Feb 23 '24 05:02 james7132

@zeenix can you check if #102 at least fixes the deadlock?

I'll check..

This took a quick glance at zbus and it isn't using Executor::run in any way, so this localizes the problem to just how ticking works.

Actually, it is. :)

zeenix avatar Feb 23 '24 12:02 zeenix

@zeenix can you check if #102 at least fixes the deadlock?

I'll check..

At least all the zbus tests pass on both Linux and Windows with your branch.

zeenix avatar Feb 23 '24 12:02 zeenix

With release v1.9.1, we've reverted the thread-local queue changes. I intend to add these back; however we have to be very careful about corner cases that are also introduced by e.g. ticking the executor instead of running it.

notgull avatar Mar 30 '24 04:03 notgull

Unfortunate, but thanks for addressing this.

james7132 avatar Mar 30 '24 09:03 james7132