Philip Chimento authored Aug 30, 2017 and Philip Chimento committed Sep 07, 2017

gjs_context_eval_file() was returning false but not setting the error
parameter if the file did not exist.

That check makes no sense anyway, since it is also done by
g_file_load_contents(). Therefore we remove it and take the opportunity
to refactor the function to use autoptrs.

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Given a std::vector<T> vec, &vec[0] is undefined behaviour when the
vector is empty. C++11 provides a vec.data() method which does the same
thing safely.

This is an opportunity for a small refactor; the free function didn't
actually free the vector, so it should be renamed "clear", and it can
just take in the vector instead of a pointer to the data. We can also
keep all the memory management of the vector in the same place, in
object_instance_init().

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Philip Chimento authored Aug 31, 2017 and Philip Chimento committed Sep 07, 2017

This pattern was leaked; according to the documentation the caller owns
the return value of cairo_pop_group().

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Philip Chimento authored Aug 31, 2017 and Philip Chimento committed Sep 07, 2017

Using g_option_context_parse() on anything but the main function's argc
and argv will leak memory. Instead, use g_option_context_parse_strv()
which is intended for use with GStrv-style allocated arrays.

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Philip Chimento authored Aug 30, 2017 and Philip Chimento committed Sep 07, 2017

It's undefined behaviour to call abs() on (int64_t)(-2^63) because 2^63
can't be stored in an int64_t. Add a special case for this value.

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Assigning out-of-range values to variables of these types is undefined
behaviour, and was caught by UBSan on Clang.

https://bugzilla.gnome.org/show_bug.cgi?id=786995

Philip Chimento authored Aug 22, 2017 and Philip Chimento committed Sep 07, 2017

Caught with Valgrind.

While touching this code, reindents it and fixes code style.

https://bugzilla.gnome.org/show_bug.cgi?id=786668

It's not possible to stop tracing an object in the middle of GC. However,
by using JS::ExposeObjectToActiveJS(), it is possible to mark an object
as reachable for the duration of one GC. This is exactly what we need for
closures, to keep the closure's callable object from disappearing while
GC is going on.

https://bugzilla.gnome.org/show_bug.cgi?id=786668

https://bugzilla.gnome.org/show_bug.cgi?id=785657

This turned out to cause a lot of problems; it has been responsible for
almost all of the crashes in gnome-shell since 1.48. We revert back to
the original code modulo a few improvements that have been made along the
way.

This state at least does not crash in the test suite, although it is
definitely not correct since it breaks SpiderMonkey's garbage collector
pre barrier verification mode (the reason the change was made in the
first place.) That still must be fixed using a different approach.

This partially reverts commits [41b78ae5], [db3e3877], [bace9089],
[9eb4a2ba], [2593d3d4], and [334ba969].

https://bugzilla.gnome.org/show_bug.cgi?id=786668

https://bugzilla.gnome.org/show_bug.cgi?id=785657

This isn't used at all. Backported from master in order to apply fixes
for closure invalidation.

https://bugzilla.gnome.org/show_bug.cgi?id=786668

https://bugzilla.gnome.org/show_bug.cgi?id=785657

When a closure is invalidated during garbage collection, we can't free it
immediately because you can't stop tracing JS objects in the middle of
garbage collections. Instead we defer the free to an idle handler.

Previously, we kept track of the idle handler ID inside the closure's
ConnectData structure. However, it was possible for an idle handler to be
scheduled and the closure subsequently freed when the GObject itself was
freed. That meant that when the JS wrapper object was finalized, there
was no way to access the idle handler ID to remove it, so the idle
handler would still run, which meant use-after-free and occasionally a
crash.

This patch keeps track of pending idle handler IDs inside the JS wrapper
object's private structure, instead of the ConnectData structure, so that
all pending handlers are definitely removed when the JS wrapper object is
finalized.

https://bugzilla.gnome.org/show_bug.cgi?id=783935

Closures trace the function object that they call on, in order to keep
the function alive during garbage collection. When the closure is
invalidated, we break that link so the function can be garbage collected,
but we must do so in an idle function, since it is illegal to stop
tracing a GC-thing in the middle of GC.

However, this caused a possible use-after-free if the closure was
scheduled to stop tracing the function object, but the last reference on
the closure was dropped before the idle function could be run.

Similar to the recent fix in gi/object.cpp [commit 2593d3d4], this avoids
use-after-free by cancelling any pending idle function in the finalize
notifier, and dropping the function object immediately.

https://bugzilla.gnome.org/show_bug.cgi?id=783935

https://bugzilla.gnome.org/show_bug.cgi?id=783214

In object_instance_finalize() we get two kinds of signal connections:
ones that are still connected, which we need to invalidate; and ones that
have just been disconnected and invalidated but whose invalidate idle
function has not yet run.

The second kind we just need to disconnect the idle handler and free the
ConnectData. We should not try to remove the invalidate notifier, since
it has already run. The first kind, we must remove the invalidate
notifier so that no idle invalidation is scheduled, and then invalidate
them, and also free the ConnectData.

Previously we were mixing the two and removing notifiers from both kinds.
This led to warnings about removing uninstalled invalidation notifiers.

https://bugzilla.gnome.org/show_bug.cgi?id=783951

The idle_invalidate_id member was being filled with garbage, leading to
errors later where nonexisting (or worse, existing) source IDs were being
removed.

https://bugzilla.gnome.org/show_bug.cgi?id=783951

In the case of a closure, the GjsMaybeOwned object is embedded as part of
struct Closure. The context destroy notify callback will invalidate the
closure, which frees the GjsMaybeOwned object, causing a use-after-free
when the callback returns and calls reset().

In practice we did not need to call reset() after the callback returns;
all existing callbacks already call reset(). This patch adds a
requirement that the callback *must* call reset(), and only calls it
internally if there was no callback set.

https://bugzilla.gnome.org/show_bug.cgi?id=781799

Objects trace their signal connections in order to keep the closures
alive during garbage collection. When invalidating a signal connection,
we must do so in an idle function, since it is illegal to stop tracing a
GC-thing in the middle of GC.

However, this caused a possible use-after-free if the signal connection
was invalidated, and then the object itself was finalized before the idle
function could be run.

This refactor avoids the use-after-free by cancelling any pending idle
invalidations in the object's finalizer, and invalidating any remaining
signal connections in such a way that no more idle functions are
scheduled.

https://bugzilla.gnome.org/show_bug.cgi?id=781799

To avoid a rare SpiderMonkey warning. Unreviewed.

https://bugzilla.gnome.org/show_bug.cgi?id=781219

Philip Chimento authored Apr 25, 2017 and Philip Chimento committed May 06, 2017

On GCC, we get warnings about the inline functions causing too much code
growth. However, we do want these functions always inlined, since they're
in a header file.

Previously we dealt with the warnings by increasing --param
inline-unit-growth, but that causes other warnings on Clang since Clang
doesn't have that parameter. I also noticed that it requires different
values on other versions of GCC, so it's not a very good solution.

Instead, we use the always_inline attribute on GCC, which causes the
functions to be inlined regardless of the compiler's inlining heuristics.

https://bugzilla.gnome.org/show_bug.cgi?id=780350

Claudio André authored Apr 26, 2017 and Philip Chimento committed May 06, 2017

It is possible to use a static analysis tool to improve all scripts robustness and safety.

https://bugzilla.gnome.org/show_bug.cgi?id=781806

Claudio André authored Apr 26, 2017 and Philip Chimento committed May 06, 2017

It is possible to use a static analysis tool to improve all scripts robustness and safety.

https://bugzilla.gnome.org/show_bug.cgi?id=781806

Tom Schoonjans authored Apr 24, 2017 and Philip Chimento committed May 06, 2017

Required for compiling with clang on macOS.

https://bugzilla.gnome.org/show_bug.cgi?id=780350

Cosimo Cecchi authored Nov 28, 2016 and Philip Chimento committed May 06, 2017

In that case, return an empty array.

(Different from its counterpart on master: test removed because it would
require gobject-introspection 1.53)

https://bugzilla.gnome.org/show_bug.cgi?id=775679

Debarshi Ray authored Apr 12, 2017 and Philip Chimento committed Apr 19, 2017

The delay property is often not specified which leads to this with
SpiderMoney 38:
JS WARNING: [resource:///org/gnome/gjs/modules/tweener/tweener.js 514]:
  reference to undefined property obj.delay

Let's do the same for 'time' to be on the safe side.

https://bugzilla.gnome.org/show_bug.cgi?id=781219

I finally figured out why my highlighting was always messed up; the mode
lines in all these files said "C" whereas they were C++ or JS files. This
was in such a blind spot that I even copied it to new files that I had
created in the meantime...

Unreviewed.

Philip Chimento authored Apr 13, 2017 and Philip Chimento committed Apr 19, 2017

Guide for developing on GJS, explaining how to set up and run the various
developer tools you'll need.

Also, the README is pretty unfriendly, starting as it does with an
admonishment to make sure to write "sane" code. Improve that a bit.

https://bugzilla.gnome.org/show_bug.cgi?id=781297

Georges Basile Stavracas Neto authored Apr 19, 2017 and Philip Chimento committed Apr 19, 2017

Inside update_heap_wrapper_weak_pointers(), we disassociate the
object while iterating over weak_pointer_list. It is possible,
however, that that disassociating the object causes the weak
pointer to be disposed, which will then call wrapped_gobj_dispose_notify(),
which will modify weak_pointer_list again, while the update_heap...
function is still iterating.

This will left weak_pointer_list with a dangling pointer and
will cause a crash.

Fix that by storing the to-be-disassociated objects in a temporary
list, wait for the iteration over weak_pointer_list finish, and
then disassociate it.

https://bugzilla.gnome.org/show_bug.cgi?id=781194