The `ThreadPoolLooper` should increment `m_busy_count` before attempting
to access the global queue. Otherwise, there exists a possible race
condition where `wait_for_all` checks the exit conditions before the
looper increments `m_busy_count` but after it empties the `ThreadPool`
queue.
Next, incrementing / decrementing `m_busy_count` is moved to be the
responsibility of `ThreadPoolLooper`. Otherwise, it is possible that
decrementing `m_busy_count` in the caller of `Looper::next` causes
`m_busy_count` to underflow if the call to `Looper::next` returns
before incrementing `m_busy_count`.
When adding tests for `ThreadPool` a handful of deadlocks can be
observed when worker threads wait on `m_work_available`.
The first deadlock is in the destruction of `ThreadPool` where it
is possible for a worker thread to be in the process of acquiring
`m_mutex` when the broadcast to `m_work_available` in the
destructor happens. This causes the destructor to hang on joining the
thread which is now perpetually waiting on `m_work_available`. This is
solved by repeatedly broadcasting on `m_work_available` until the thread
to join exits.
The second deadlock occurs when the final signal to `m_work_done` is
missed by the wait in `wait_for_all`. At this point all workers are in
the hot loop of attempting to get work from the work queue, however
since there is no work remaining all workers end up waiting on
`m_work_available`. At this point the `wait_for_all` call is also
waiting on `m_work_done`, which will never be signalled again as all
workers are waiting on `m_work_available`.
This requires 2 changes to fix, the first is that workers will signal
`m_done_work` before waiting on `m_work_available`. The second change is
to acquire `m_mutex` before checking the wait conditions as done when
using `wait_while`.
This allows us to use HeapFunction all of the way down, allowing us
to remove the Handle usage in after_session_callback for
create_new_child_navigable.
...For the completion steps. This is quite nice, as we can simply
capture this in the heap function where it is used instead of
needing to establish a new root.
Note that with these changes, to represent 'an empty algorithm', we now
use a null HeapFunction and do not invoke the steps.
This requires pulling in some of the STL, but the result is that our
iterator is now STL Approved ™️ and our containers can be
auto-conformed to Swift protocols.
USVString attributes Now replace any surrogates with the replacement
character U+FFFD and resolve any relative URLs to an absolute URL. This
brings our implementation in line with the specification.
The Web Platform Tests runner requires that some hostnames point to
localhost when running the tests locally. We now append these hostnames
to `/etc/hosts` if they aren't already present.
This brings back the old behaviour of Value::to<short>() (and other
similar calls), which WASI depends on.
To make sure all similar issues are caught in the future, this commit
also introduces an static assertion in Value::to().
The Initialize* AOs for Intl formatters were removed some time ago, and
the formatter construction steps are now inlined in the constructors
themselves. InitializeNumberFormat was the one remaining initializer we
still had laying around.
This constructor has undergone a handful of editorial changes that we
fell behind on. But we weren't able to take the updates until now due to
a spec bug in those updates. See:
https://github.com/tc39/ecma402/commit/3f029b0
The result is that we can remove the inheritance of Intl::DateTimeFormat
from Unicode::DateTimeFormat; the former now contains the latter as an
internal slot.
We previously had 4 single-instance StyleValues for these keywords.
CSS-Typed-OM expects them keywords to be exposed as CSSKeywordValue, so
it's simpler to treat them the same. The single-instance behaviour is
kept by having StyleValue::create() use a cached instance for each of
these.
For a long time, we've used two terms, inconsistently:
- "Identifier" is a spec term, but refers to a sequence of alphanumeric
characters, which may or may not be a keyword. (Keywords are a
subset of all identifiers.)
- "ValueID" is entirely non-spec, and is directly called a "keyword" in
the CSS specs.
So to avoid confusion as much as possible, let's align with the spec
terminology. I've attempted to change variable names as well, but
obviously we use Keywords in a lot of places in LibWeb and so I may
have missed some.
One exception is that I've not renamed "valid-identifiers" in
Properties.json... I'd like to combine that and the "valid-types" array
together eventually, so there's no benefit to doing an extra rename
now.
