The last fix had a panic where a guard should have been allowed to be
created. Change callbacks detect when they are about to be fired from
themselves, and prevent the deadlock.
Very hard to intentionally reproduce, thankfully the second time I saw
it I was in the debugger and was able to reason about the code path that
could have gotten in that particular state. Comment explains the actual
situation.
- synchronize_platform_window is now called prior to the first redraw.
This allows the `visible` attribute to be changed from false to true.
- Some window attributes are automatically set based on the incoming
dynamic.
- Some initial window values are delayed until after the first layout to
minimze "noisy" values.
All of these changes now allow a window that is resize_to_fit to be
initially hidden and show itself centered after being initially resized
without any flashing or on-screen movement/resizing.
During the change callback process, unlocked is called to allow the
change callbacks to run while the dynamic is unlocked. The error with
the previous version of this code is that the during_callback_state was
always overwritten when being returned. The problem is that another
thread could currently have the mutex locked and could have stored its
own state -- which can happen if two threads are both trying to invoke
change callbacks at the same time.
By moving the state saving and reloading to only happen when the mutex
guard is acquired, we can ensure that interleaving threads will work
correctly.
Refs #153
This turned out to be "needed" by the debug window due to how dynamic
locking was nested when a for_each call was being invoked. To keep the
code simple, for_each_subsequent was added.
map_each previously was written such that if a chain of mappings fed
each other, a deadlock could occur because while the first one was
mapped, the second callback gets invoked and tries to update the first
value while it's still being held.
This refactor switches from std Mutex to parking_lot, allowing me to
remove a workaround for needing to run drop callbacks in a separate
thread during the drop of a DynamicGuard.
In addition to that change, the lower level `map_generational` calls now
take a DynamicGuard as their parameter. This allows these functions to
drop ownership of the referenced data during the callback.
The map_each implementation takes advantage of this by ensuring that the
guard is dropped before set is invoked, minimizing potential lock overlaps.
With this refactor, some old code of mine with complex validations now works
again.
Somehow I missed that my changes for weak callbacks broke the theme
editor. I thought I had it working with the try_get changes, but I
discovered several flaws in this approach.
In the end, ownership has been transferred to the CallbackHandle, and a
CallbackHandle can relinquish its reference to create weak graphs. This
is how weak_clone now works.
Closes#98
This finishes my initial refactoring of the dynamic system to add
support for several dataflows including:
- Pure data sources that can be implemented using an `Owned<T>` at the
root of a graph of `Dynamic<U>`/`DynamicReader<U>`s.
- Read-only data sinks. I thought this would be more useful across other
widgets, but in general, Progress and Label seem like the only types
that this applies to currently.
- The ability to mix/match Dynamic/DynamicReader in tuple-based
for_each/map_each.
Refs #98
This refactor overhauls the reactive system to move all the reactive
methods to traits. The side effect of this change is that now
DynamicReader's API is the same as Dynamic's API, but because it only
implements Source<T>, DynamicReader does not offer any mutation
functions.
While it's unfortunate to have more traits to include to use Cushy, this
seems like the best option, and it offers a path to try to integrate
this into the tuple ForEach/MapEach traits. Unfortunately, my attempt at
doing those in this set of changes led to issues specifying generic
associated lifetimes for the DynamicGuard. But, I was also in the middle
of this larger refactoring, so it might be that a fresh attempt will
succeed.
After adding weak_clone, I realized that all map functions should use
weak references anyways. In the process of implementing this, I ran
tests a lot and found other edge cases where I hadn't properly reasoned
about Drop behavior.
While some of the state cleanup is a bit overkill at the moment, this is
in anticipation of wanting a WeakDynamicReader-like type.
The animation doctest randomly failed on me this morning, and I isolated
it to the order of the two mutexes. There's no reason to hold the
deadlock mutex for more than the check for deadlock, so I switched which
mutex the condvar is synchronizing using to ensure that another thread
couldn't cause a deadlock by dropping a DynamicMutexGuard while the
block_until_updated thread is performing its state check.
There was a small window between when notify_all and the strong count
for the Arc is decreased that a DynamicReader could observe the strong
count still being greater than the reader count, but the drop thread
just hasn't proceeded far enough.
Now the on_disconnect is stored in an option, and its presence denotes
that the disconnect logic has not fired yet. DynamicReader now checks
on_disconnect in addition to the strong count.
This cascaded into a lot more work than expected. However, in general,
if one clones a `WidgetInstance` and shares it between two windows, it
should now work. Widget authors must ensure that when they cache
information, they do so with either a `WidgetCacheKey` or use a
`WindowLocal<T>` if per-window state is desired.
This is demonstrated in the debug-window example, where the counter of
open windows is next to a clone of the same button from the main window
that opens a new window.
This sounds like a regression, but it was masking a "race condition".
DynamicGuard runs the change callbacks on drop in a background thread.
Validations was using the guard to not have to lock twice.
This led to an issue where the invalid count might be non-zero due to
the callbacks not being invoked, preventing the closure from being
invoked even though there are no validation errors.
Introducing compare_swap gives a higher-level API for Validations to
use, and it also ensures the callbacks are able to be run in the current
thread.
Closes#97
There was a potential race condition described in #97 that I realized I
had seen occasionally when interacting with an element that was
currently being animated. These were in complex situations, so I thought
I had a situation that could have legitimately caused the warning.
However, this warning is preventing a very specific coding "error", and
that program did not have it. The existing implementation would
potentially prevent one thread's change from invoking its callbacks
because another thread was already executing its callbacks.
This change moves that state into a Mutex/Condvar pair that allows
detecting reentry while allowing other threads to block until its their
turn. When it becomes their turn, they can check whether the callbacks
were invoked with the current value or not to prevent callbacks from
being invoked in quick succeession with the same value by multiple
threads.
Debug printing widgets was quite verbose. While developing a widget, you
often want to see a full debug printout, but this feature assumes that
debug printing a WidgetInstance should show a summary of the widget, not
a full debug printout containing cached glyph information of every
label.
By default, summarize just calls Debug, but this extra layer allows
widgets to provide a more condensed summary and exclude details like
caches.
Originally, adding dbg!() around the theme example's UI yielded a
whopping 20,324 lines of text. The summary code only prints 3,858
lines.
Installing a callback now returns a CallbackHandle. All map-style APIs
install this handle automatically on the created dynamic, which keeps
the callback installed until the dynamic is freed. All other APIs
return the handle for the caller to either call persist() or store
somewhere.
Now, the dynamic system can be used for application-long data with
almost no fear of leaking data due to how callbacks are being installed.
Technically cycles are still possible by moving clones into the
callbacks, so a WeakDynamic type might be worth exposing.
- Caching font family resolution to avoid scanning the database over and
over. The db should still be cached, but this makes repeated setting
free.
- into_switcher rename for Dynamic<WidgetInstance> to avoid conflicting
with Switchable::switcher()
- Dynamic debugging is less verbose
- IntoDynamic<Validation> for Result<T,E>
- Input no longer blinks cursor when disabled.
