Rename the type to something more sensible and change it to remove the
last remanent of Eo1. This fixes a fixme that has been there for a
while.
The type doesn't really matter, it just looks nicer with the va_list.
This commit implements a sort of CoW for the vtables. The vtables are
usually just linked to and refcounted. When we need to change them we
allocate new ones and copy them over so we can write to them.
I wrote some code to measure the effectiveness of this change. When
running elementary_test (and immediately exiting) I saw that out of the
total number of vtable chains (561) that were needed by the classes in
the EFL, 79 (14.08%) were reused. Considering that I had to add
refcounting (unsigned short, but let's consider it's the size of a word
because of alignment), I would calculate the saving as such (in bytes):
Number of items in a chain (refcounted block): 32
32 bit:
sizeof(chain_node) = 8
Mem wasted on refcounting: 561 * 4 = 2244
Mem saved because of sharing: 79 * (32 * 8) = 20224
Total save: 17980 bytes
64 bit:
sizeof(chain_node) = 16
Mem wasted on refcounting: 561 * 8 = 4488
Mem saved because of sharing: 79 * (32 * 16) = 40448
Total save: 35960 bytes
Wow, we use a lot of memory in Eo classes, I'm sure we can
save even more if we put our hearts into it (change the shareable units
to be smaller to increase the chance of sharing).
This is internal and doesn't affect API/ABI so we can change this even
further with time.
This also improves efl_object_override(). This should now be quite
memory efficient (don't abuse, but it's not a big hogg as it was), so
feel free to abuse that one and rely on it in API.
@feature
ok. so here's the issue at least now. we have eo objects in the canvas
and they have a refcount of 2 user_refcount is 0. the calls stack does
NOT show we are calling callbacks at that time on these objects. they
are not in the backtrace (the canvas is, the objects themselves are
not).
SOMETHING is keeping 2 eo "internal" refs on these objects and i have
no idea what/how/who. it's a royal pain in the butt to find out as the
only way is lots and lots of logging and you get drowned in the
logging...
so what I have now done is a super ugly workaround that detects these
zombie objects that refuse to die and just FORCES them to die when the
evas canvas frees and clears out layers.
ac10a00acc doesn't really cause the
issue, it just brings it out in the open for all to see far more
easily. but something is deeply wrong SOMEWHERE with SOME objects and
our refcounts.
this fixes T4187
This is a (minor) API & ABI break in Eo.h!
I say minor as eo_override shouldn't be used yet (EO is unstable
and this patch includes all the use cases in EFL).
I'm not very happy about the new form of the macro, but it avoids
two things:
- passing in a struct (valid in C, but never used in EFL)
- using a GCC construct to create structs on the fly
It was inspired by the event array define, but I don't think
we need the runtime memcpy here.
See also:
https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html
eo_override would leak the vtable if called multiple times, this
fixes that. Also, it is now possible to revert back to the original
class' vtable by passing in { NULL, 0 }
I believe it is thus possible to incrementally override more
functions on an object. Absolutely not recommended, but should work.
But it is not possible to selectively revert back to the original
class implementation on a single method. Use eo_super for that,
or revert back the entire object overrides.
PS: Is it normal that we pass in a struct? We never do that in EFL...
When using eo_add_ref, it was increasing the refcount before the user
context in the addition has fully ended. This means the object had its
reference increased while still not finalized, which means it was
sometimes passed with an increased refcount to unsuspecting class code.
The correct behaviour is to increase the reference count just before
returning the object to the user at the end of eo_add so the reference
count is only increased for whoever asked for it.
Breaks ABI!
@fix
This commit changes the way refcount is dealt with internally. Before
this commit, there was one refcount shared between Eo internals and
users. Now there is a refcount for eo operations (like for example,
function calls) and one for user refcount (eo_ref).
An example bug that this protects against (which is seemingly rather
common) is:
some_eo_func(obj);
// Inside the implementation of that func:
pd->a = 1; // The object's private data
eo_unref(obj); // To delete the object
eo_unref(obj); // A big one extra unref
pd->a = 2; // Segfault, this data has already been freed
This is a feature, but really just a fix for a class of bugs.
@feature
I was getting a crash in eo_shutdown, inside
_efl_event_pointer_class_destructor as I was calling eo_del
from there. But the parent class was already destroyed.
Assuming class IDs can only go up, and child classes are only
instanciated after all their parents, it is safer to call the
class destructors in reverse order.
Obviously, still pretty sure eo_del() in a class_destructor
is not a good idea...
This was needed when the eo composite object was still in beta. Since commit
d7c45e41d4 this is no longer the case. No beta
part left in eo base so we can safely remove this define.
We used to keep a reference to the parent object and have it in the call
structure although we were actually calling the function on the embedded
object. This was needed because we wanted to unref the parent correctly.
This was incorrect (and marked as a hack) and now I finally gotten
around to implementing the (amazingly simple) fix to remove this
workaround.
Essentially we just ref the comp object, unref the parent, and let the
normal eo call flow to unref the comp object correctly later on, like it
would have unreffed the extra ref we had for the parent.
This reverts commit 546ff7bbba.
It seems that eo_del() is useful and removing it was creating bugs.
The issue is that the way we defined parents in eo, both the parent and
the programmer share a reference to the object. When we eo_unref() that
reference as the programmer, eo has no way to know it's this specific
reference we are freeing, and not a general one, so in some
circumstances, for example:
eo_ref(child);
eo_unref(child); // trying to delete here
eo_unref(container); // container is deleted here
eo_unref(child); // child already has 0 refs before this point.
We would have an issue with references and objects being freed too soon
and in general, issue with the references.
Having eo_del() solves that, because this one explicitly unparents if
there is a parent, meaning the reference ownership is explicitly taken
by the programmer.
eo_del() is essentially a convenience function around "check if has
parent, and if so unparent, otherwise, unref". Which should be used when
you want to delete an object although it has a parent, and is equivalent
to eo_unref() when it doesn't have one.
so... i got this ... callback calls callback calls something calls
callback that deletes the original object at the top so when it comes
back ... things die as the object was destructed. in removing eo_do()
we removed the ref/unrefs that went with it. so this uses the
_EO_API_BEFORE_HOOK and _EO_API_AFTER_HOOK to call exposed "internal"
public functions _eo_real_ref() and _eo_real_unref().
this fixes a new segv i've noticed in several e dialogs where hitting
close does the above via callbacks and closes the window etc.
This change lets you override the functions of objects so that those
functions will be called instead of the functions of the class. This
lets you change objects on the fly and makes using the delegate pattern
easier (no need to create a class every time anymore).
You can see the newly added tests (in this commit) for usage examples.
@feature
We used to have eo_del() as the mirrored action to eo_add(). No longer,
now you just always eo_unref() to delete an object. This change makes it
so the reference of the parent is shared with the reference the
programmer has. So eo_parent_set(obj, NULL) can free an object, and so
does eo_unref() (even if there is a parent).
This means Eo no longer complains if you have a parent during deletion.
This was done following a feature request by @raster. There was no real
reason for it not to be an eo function and this gives us more
flexibility.
The reason why this done was to provide a way for classes to do special
things when an object deletion was requested, for example in the case of
Evas, hide the object.
so memory for class id -> ptrs is mallocs. this means it likely will
be next to other memory malloced. which means overrunning memory
someone mallocs could walk into the class table and corrupt it. we put
eo ids in mmaped regions to avoid this if possible in case of buggy
code. let's do it for classes too.
this also now allocs in larger chunks. for mmap its in page chunks
(which can hold either 1024 or 512 classes depending on 32 or 64bit).
reallocs still work if mmap is not there and we do them in chunks of
128 classes (it seems that we start at about 70 or so classes atm when
elm_test starts and it grows to ~100 or let's do 128 as that's pretty
much our base as a power of 2 and we now dont realloc much).
I'm reverting this because according to jpeg it was possibly fixed in
5284b62e93.
I reverted this patch after his fix and followed his reproduction cases
and it seems that his second patch does indeed fix this issue so this
patch is no longer needed.
This reverts commit 0862b9d083.
It seems that calling a @class function with an EO object
(that was not the required Eo_Class) lead to a situation
where func->func was NULL. And that meant a crash after
call_resolve.
The proper fix is to properly call a @class function with a
class object.
The current eo_add uses a (very useful) gcc extension that is only
available in gcc compatible compilers (e.g clang). Until this commit we
just temporarily ignored this fact. This adds a fallback implementation that
can be used interchangeably with the non portable one. This means that the
same binary can call either at any point in time and the code will work.
Breaks ABI.
Imagine this. You have an object. You pass this object handle as a
message to another thread. Let's say it's not a UI object, so
something you might expect to be able to be accessed from multiple
threads. In order to keep the object alive you eo_ref() it when
placing the message on a queue and eo_unref() it once the message is
"done" in the other thread. If the original sender unref()ed the
object before the message is done, then the object will be destroyed
in the reciever thread. This is bad for objects "expecting" not to be
destroyed outside their owning thread.
This allows thius situation to be fixed. A constructor in a class of
an object can set up a delete interceptor. For example if we have a
"loop ownership" class you multi-ple-inherit from/use as a mixin. This
class will set up the interceptor to ensure that on destruction if
pthread_self() != owning loop thread id, then add object to "delete
me" queue on the owning loop and wake it up. the owning loop thread
will wake up and then process this queue and delete the queued objects
nicely and safely within the "owning context".
This can also be used in this same manner to defer deletion within a
loop "until later" in the same delete_me queue.
You can even use this as a caching mechanism for objects to prevernt
their actual destruction and instead place them in a cached area to be
picked from at a later date.
The uses are many for this and this is a basic building block for
future EFL features like generic messages where a message payload
could be an eo object and thus the above loop onwership issue can
happen and needs fixing.
This adds APIs, implementation, documentation (doxy reference) and tests.
@feature
The syntax is described in: https://phab.enlightenment.org/w/eo/
Summary:
eo_do(obj, a_set(1)) -> a_set(obj, 1)
eo_do_super(obj, CLASS, a_set(1)) -> a_set(eo_super(obj, CLASS), 1)
eo_do_*_ret() set of functions are no longer needed.
This is the first step, the next step would be to also fix up eo_add()
which currently still uses the old syntax and is not 100% portable.
@feature
Until now it wasn't allowed/possible to init (eo_init) eo after it has
been shut down (eo_shutdown). This commit fixes that, so now that is
fully legal to have as many init/shutdown cycles as you want.
There was a previous workaround for this issue:
e47edc250d.
This should allow more flexibility when using the EFL in loadable
modules and in various other scenarios.
The problem is that the class_get() functions cache the previously
created class for efficiency, but the class is freed if eo is shut down,
so the cached pointer is actually invalid.
The solution to the problem was to maintain a generation count
(incremented every time we shut down eo), and compare that to a locally
saved version in class_get(). If they don't match, recreate the class,
as it has already been freed.
@feature
This commit was a workaround to let us shutdown and then init eo without
any issues. It leaks and it's wrong. This will properly be fixed in the
next commit.
This reverts commit e47edc250d.
This is not really needed, I just did it to make it easier for coverity
(and future static analysers) to understand that the class id doesn't
need to be accessed with a lock.
CID1341854
Currently, eo_shutdown can't work.
Every Eo_Class ID is stored inside its class_get() function as a
static variable. This means any call to class_get() after eo_shutdown()
(even if eo_init was done properly) will lead to using an invalid ref
for the class id. In other words, the class is not valid anymore,
and objects can't be created.
Resetting the pointer to NULL would be possible, if we passed it
during the class creation. But this would lead to potential crashes
if a class was created from a now dlclosed library.
The only solution I can envision here is to check that class_get
actually returns a valid ref with the right class name. Most likely
the performance impact is not acceptable.
This fixes make check for me (with systemd module for ecore).
This is faster in most cases, and to be honest, should be much faster
than it is. I don't understand why there's no better directive to mark a
variable as *really* important thread storage that is used all the time.
We don't really need the eo_id most of the time, and when we do, it's
very easy to get it. It's better if we just don't save the eo_id on the
stack, and just save if it's an object or a class instead.
It seems that the idea behind that optimisation, is to save object data
fetching when calling functions implemented by the object's class inside
functions implemented by the object's class. This should be rare enough
not to worth the upkeep, memory reads and memory writes, especially
since for all cases apart of mixins (for which this optimisation won't
work for anyway), the upkeep is more costly than fetching the data
again.
Tom Hacohen
Carsten Haitzler
Jean-Philippe Andre
Stefan Schmidt
Vincent Torri