mirror of
https://github.com/brl/mutter.git
synced 2025-06-13 00:39:30 +00:00
7ddbcd1fd2
d42f1873fc introduced a semi circular
reference between the CoglFramebuffer, and CoglJournal, where
CoglJournal would keep a reference on the CoglFramebuffer when there
were any entries in the journal log.
To avoid risking leaking these objects indefinitely, when freeing
objects without doing anything that triggered a flush, CoglFramebuffer
had a "filter" on cogl_object_unref() calls, which knew
about under what conditions CoglJournal had a reference to it. When it
could detect that there were only the journal itself holding such a
reference, it'd flush the journal, effectively releasing the reference
the journal held, thus freeing itself, as well as the journal.
When CoglFramebuffer was ported to be implemented using GObject instead
of CoglObject, this "filter" was missed, causing not only awkward but
infrequent leaks, but also situations where we'd flush journals when
only the journal itself held the last reference to the framebuffer,
meaning the journal would free the framebuffer, thus itself, in the
middle of flushing, causing memory corruption and crashes.
A way to detect this, by asserting on CoglObject reference count during
flush, is by adding the `g_assert()` as described below, which will
assert instead cause memory corruption.
void
_cogl_journal_flush (CoglJournal *journal
{
...
_cogl_journal_discard (journal);
+ g_assert (journal->_parent.ref_count > 0);
...
}
Fix this by making CoglFramebuffer the owner of the journal, which it
already was, and remove any circle referencing that was there before, as
it is not needed given that the CoglFramebuffer pointer is guaranteed to
be valid for the lifetime of CoglJournal as the framebuffer is the owner
of the journal.
However, to not miss flushing before tearing down, which is important as
this flushes painting calls to the driver that is important for e.g.
using the result of those journal entries, flush the journal the first
time cogl_framebuffer_dispose() is called, before doing anything else.
This also adds a test case. Without having broken the circular
reference, the test would fail on g_assert_null (offscreen), as it would
have been "leaked" at this point, but the actual memory corruption would
be a result of the `cogl_texture_get_data()` call, which flushes the
framebuffer, and causes the 'mid-flush' destruction of the journal
described above. Note that the texture keeps track of dependent
framebuffers, but it does not hold any references to them.
Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/1474
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1735>Outline of test categories:
The conform/ tests:
-------------------
These tests should be non-interactive unit-tests that verify a single
feature is behaving as documented. See conform/ADDING_NEW_TESTS for more
details.
Although it may seem a bit awkward; all the tests are built into a
single binary because it makes building the tests *much* faster by avoiding
lots of linking.
Each test has a wrapper script generated though so running the individual tests
should be convenient enough. Running the wrapper script will also print out for
convenience how you could run the test under gdb or valgrind like this for
example:
NOTE: For debugging purposes, you can run this single test as follows:
$ libtool --mode=execute \
gdb --eval-command="b test_cogl_depth_test" \
--args ./test-conformance -p /conform/cogl/test_cogl_depth_test
or:
$ env G_SLICE=always-malloc \
libtool --mode=execute \
valgrind ./test-conformance -p /conform/cogl/test_cogl_depth_test
By default the conformance tests are run offscreen. This makes the tests run
much faster and they also don't interfere with other work you may want to do by
constantly stealing focus. CoglOnscreen framebuffers obviously don't get tested
this way so it's important that the tests also get run onscreen every once in a
while, especially if changes are being made to CoglFramebuffer related code.
Onscreen testing can be enabled by setting COGL_TEST_ONSCREEN=1 in your
environment.
The micro-bench/ tests:
-----------------------
These should be focused performance tests, ideally testing a
single metric. Please never forget that these tests are synthetic and if you
are using them then you understand what metric is being tested. They probably
don't reflect any real world application loads and the intention is that you
use these tests once you have already determined the crux of your problem and
need focused feedback that your changes are indeed improving matters. There is
no exit status requirements for these tests, but they should give clear
feedback as to their performance. If the framerate is the feedback metric, then
the test should forcibly enable FPS debugging.
The data/ directory:
--------------------
This contains optional data (like images) that can be referenced by a test.
Misc notes:
-----------
• All tests should ideally include a detailed description in the source
explaining exactly what the test is for, how the test was designed to work,
and possibly a rationale for the approach taken for testing.
• When running tests under Valgrind, you should follow the instructions
available here:
http://live.gnome.org/Valgrind
and also use the suppression file available inside the data/ directory.