_efl_object_api_op_id_get() will query a hash for the given pointer,
however if it wasn't populated, it will return "NOOP" and we're
hopeless while debugging on what happened.
Common case is to use the incorrect method, like:
obj = efl_add(CLS1, ...);
cls2_method(obj);
Since we did not create CLS2, it won't populate its methods on the
hash, thus the lookup will return NOOP.
With this change the function now gets the target object and function
name so reports an insightful message such as:
ERR:eo file.c:123 cls2_method() Unable to resolve op for api func 0x7ff492ddea00 for obj=0x400000007e8ee1df (CLS1)
Eo pointer indirection is super nice as it avoids you to access
invalid memory, but this extra checks inhibits valgrind's own tracking
of memory lifecycle, usually it would report when the object was
created and when the object is deleted, both as stack traces.
This commits introduces logging of object creation and destruction
under its own eina_log_domain and controlled by EO_LIFECYCLE_DEBUG and
EO_LIFECYCLE_NO_DEBUG envvars. These will only be available if
compiled with EO_DEBUG, thus shouldn't cause any performance hits on
production code.
Running a bogus app with invalid efl_class_name_get() and double
efl_del() will report as below:
```sh
$ export EO_LIFECYCLE_NO_DEBUG=Efl_Loop_Timer,Efl_Promise,Efl_Future
$ export EO_LIFECYCLE_DEBUG=1
$ export EINA_LOG_LEVELS=eo_lifecycle:4
$ /tmp/bogus_app
DBG:eo_lifecycle lib/eo/eo.c:2712 _eo_log_obj_init() will log all object allocation and free
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Future'
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Promise'
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Loop_Timer'
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35a1aa0 obj_id=0x4000000002cf38ef class=0x563fa35a1450 (Efl_Vpath_Core) [0.0004]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35af8d0 obj_id=0x4000000006cf38f0 class=0x563fa35aecf0 (Efl_Loop) [0.0005]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d61a0 obj_id=0x400000007ecf390e class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0054]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d6470 obj_id=0x4000000082cf390f class=0x563fa35d0d60 (Efl_Net_Dialer_Tcp) [0.0055]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d75b0 obj_id=0x4000000086cf3910 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0056]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d8f70 obj_id=0x400000008acf3911 class=0x563fa35d7860 (Efl_Io_Copier) [0.0057]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35df980 obj_id=0x40000000a6cf3918 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0058]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35dfc30 obj_id=0x40000000aacf3919 class=0x563fa35d7860 (Efl_Io_Copier) [0.0058]
will efl_class_name_get() with invalid handle:
ERR:eo lib/eo/eo.c:1013 efl_class_name_get() Class (0x2000000000000029) is an invalid ref.
ERR:eo_lifecycle lib/eo/eo.c:1013 efl_class_name_get() obj_id=0x2000000000000029 was neither created or deleted (EO_LIFECYCLE_NO_DEBUG='Efl_Loop_Timer,Efl_Promise,Efl_Future').
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35df980 obj_id=0x40000000a6cf3918 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35dfc30 obj_id=0x40000000aacf3919 class=0x563fa35d7860 (Efl_Io_Copier) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d75b0 obj_id=0x4000000086cf3910 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d8f70 obj_id=0x400000008acf3911 class=0x563fa35d7860 (Efl_Io_Copier) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d6470 obj_id=0x4000000082cf390f class=0x563fa35d0d60 (Efl_Net_Dialer_Tcp) [0.0063]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d61a0 obj_id=0x400000007ecf390e class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0063]
will double free:
ERR:eo ../src/lib/eo/efl_object.eo.c:78 efl_del() EOID 0x400000007ecf390e is not a valid object. EOID domain=0, current_domain=0, local_domain=0. EOID generation=2cf390e, id=1f, ref=1, super=0. Thread self=main. Available domains [0 1 ]. Maybe it has been deleted or does not belong to your thread?
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e created obj=0x563fa35d61a0, class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0054s, 0.0009 ago]:
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d0ea: libeo_dbg.so+0x90ea (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6ca62: _efl_add_internal_start+0x1c2 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc95f: bogus_app+0x295f (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0ace7291: __libc_start_main+0xf1 (in /usr/lib/libc.so.6 0x7f2c0acc7000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc48a: _start+0x2a (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e deleted obj=0x563fa35d61a0, class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0063s, 0.0000 ago]:
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d8ba: libeo_dbg.so+0x98ba (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d711: libeo_dbg.so+0x9711 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6beb8: libeo_dbg.so+0x7eb8 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6c06e: _efl_object_call_end+0x4e (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc75725: efl_del+0x105 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dcd54: lt-efl_net_dialer_simple_example+0x2d54 (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0ace7291: __libc_start_main+0xf1 (in /usr/lib/libc.so.6 0x7f2c0acc7000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc48a: _start+0x2a (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e was already deleted 0.0000 seconds ago!
```
if for some reason we fail to validate a class, then we should skip
that extension. This may result in an empty vtable, then check for
that and avoid a crash.
This is very unlike to happen in practice, but I've forced some
validation errors and could get to that.
Instead of 2 sets of macro, one for HAVE_EO_ID and another without,
use a single set of macros and have the implementation of
_eo_class_pointer_get() and _eo_obj_pointer_get() to do the actual
These functions now take the source information so the logs reflect
that and not always the same function.
_eo_pointer_error() was kinda a bitch to debug as it provided a nice
breakpoint location, but did not provide a good output since the file,
line and function were always the same.
Change that to be a thin wrapper on top of eina_log_vprint(), then we
keep the breakpoint location yet provide useful information.
In that sense, change other error messages so they carry as much
information as possible.
This fixes T4907
The problem was that in efl_event_callback_add the internal array was
changed. If this was happening while a efl_event_callback_call was
happening the for loop got confused and skipped one event subscription.
Which led to a bug in e where the idler ufnction was not executed
probebly and so the canvas stayed frozen.
if there is a event the callback counter should be incremented not
decremented. This should fix a few crashes i found in edje, since edje
did not knew that a element was deletion.
Normally when debugging Eo with gdb you can just use any of the internal
eo functions to resolve the id to its internal pointer. However, when
loading a coredump you can't execute any code, not even the id resolve
code.
This change adds a gdb function that resolves the id to its pointer form
without executing any code in the process space. This plugin is
essentially the id resolve code written in python as a gdb function.
Usage:
Print the pointer:
(gdb) print $eo_resolve(obj)
$1 = (_Eo_Object *) 0x5555559bbe70
Use it directly (e.g. to print the class name):
(gdb) $eo_resolve(obj)->klass->desc.name
This plugin requires that the coredump would be loaded with the exact
same libeo.so binary (or at least one that hasn't changed eo internals),
and that the debug symbols for libeo.so would be available for gdb to
use.
Note:
This feature is incomplete and only resolves IDs that are owned by the
main thread and in the main domain. This is not a big issue at the
moment, because almost all of our IDs are like that.
@feature
so hunting another callback issue i noticed some of THE most popular
callbacks are:
1411 tick
1961 move
4157 pointer,move
7524 dirty
8090 damage
13052 render,flush,post
13052 render,flush,pre
13205 render,post
13205 render,pre
21706 recalc
21875 idle
27224 resize
27779 del
31011 idle,enter
31011 idle,exit
60461 callback,del
104546 callback,add
126400 animator,tick
as you can see callback del, add and the general obj del cb's are
right up there... so it is very likely a good idea to CHECK to see if
anyone is listening before calling the callback for these very very
very common calls.
this is ifdef'd and turned on for now. it can be turned off. it
shouldnt use more memory due to the way memory alignment works (likely
all allocations will be multiples of 8 bytes anyway) so we're using
spare unused space. the only q is - is management of the counts AND
checking them worth it in general? it's really hard to tell given we
dont have a lot of benchmarks that cover a lot of use cases... it
doesnt seem to slow or speed anything up much in the genlist bounce
test... so i think i need something more specific.
@optimize
i found a massive slowdown that over time ended up with 10000's of
cb's in objects like the ecore loop object. this fixes that by
ACTUALLY flagging event deletions waiting to be true rather than false.
When resolving a function for an object the object would get reference,
and then in some failure cases won't be freed.
I suspect this is a regression following the reshuffling that was done
in that function recently.
Thanks to zmike for investigating and reporting this.
Fixes T4740
@fix
This happens with shared objects.
The situation seems to be:
1. object has composited object a of class A in thread 1
2. call something on object a from thread 2, deadlock
In fact, do anything from thread 2 on a shared object and you deadlock.
this moves a lot of error case handling into goto's so the code gets
out of the hot path and this should help expecially since variou
smacros do things like:
do { char buf[256]; sprintf(buf, fmt, ptr); _eo_pointer_error(buf); } while (0)
_Efl_Class *klass; \
do { \
klass = _eo_class_pointer_get(klass_id); \
if (!klass) { \
_EO_POINTER_ERR("Class (%p) is an invalid ref.", klass_id); \
return ret; \
} \
} while (0)
so putting quite a chunk of code inside a rare "if this errors"
handler that will cause l1 cache misses and this we don't want, thus
moving stuff in eo core out of hot paths to cut down on overhead. yes
it might not be pretty but it's kind of the right thing at such a core
level of efl. this also does the same to the eo base class as this is
also going to be relatively hot given it's the core of every other
object.
so there were a few issues. one we had a spinlokc on the eoid table
for shared objects AND then had a mutex for accessing those objects
(released on return from any eo function). BUT this missed some funcs
like eo_ref, eo_unref and so on in eo.c ... oops. so fixed. but then i
realized there was a race condition. we locked the eoid table then
unlocked with our pointer THEN locked the sharted object mutex ...
then unlocked it. that was a race condtion gap. so we should share the
same lock anyway - if it's a shared object, grab the shared object
mutex then do a lookup and if the lookup does not fail, KEEP the lock
until it is released by the return from eo function or by some special
macro/funcs that released a matching lock. since its a recursive lock
this is all fine. as its also a universal single lock for all objects
we just need the eoid to know if it's shared and needs locking based
on the domain bits. so now do this locking properly with just a single
mutex, not both a spinlock and mutex and keep the lock around until
totally done with the object. this plugs the race condition holes and
goes from 1 spinlock lock and unlock then a mutex lock and unlokc to
just a single mutex lock and unlock. this means shared objects are
actually truly safe across threads and only have the overhead of a
single recursive mutex to lock and unlock in every api call.
as per other recent benchmarking, moving rearely run code (in this
case code to init the op etc.) out of the l1 cacheline prefetch inot a
blob of code at the end of the function where we goto and goto back
again should provide decent-ish speedups for the resolv cache in
avoding this code. yes it makes the code less pretty to read but at
this really low level hot path ... evil things must happen to get the
speed we want/need.
Summary:
as told in _eina_stringshared_key_cmp in eina_hash.c:
originally we want to do this:
return key1 - key2;
but since they are ptrs and an int can't store the different of 2 ptrs in
either 32 or 64bit (signed hasn't got enough range for the diff of 2
32bit values regardless of their type... we'd need 33bits or 65bits)
So changing this to the same logic.
Reviewers: tasn, raster
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4298
whenan eoid lookup fails, now print a lot of information on the issue
like the actual id, generation of the id, if its a class or object
(the class bit), if its ref or super bit is set, the actual id (which
includes the table heirachy), which thread id it is, what domain the
object id is and the current and local domains as well as what domains
are mapped in.
This change lets us remove a field from the structure that leads to
around 20KiB more of saving in private dirty pages in elementary.
This also looks a bit better and feels a bit cleaner.
Breaks API and ABI.
we now just lost another bit from generation count. down to 6 in 32bit
and 26 in 64 bit. this sucks but is necessary. now we are using the
bits just below ref and super bits the code was just maskign off the
next bit as a class marker. this was so so so so wrong. it was the ide
table space. we just never used numbers high enough to start using it.
since i added domain there now those bits can be used easily with
thread domain or other domain. argh! existing eo bug found and fixed.
annoying! :) i added another #define there just to be clear we use
that bit for classes.
Before this commit, function overrides were explicit. That is, you'd
have to explicitly state you were overriding a function instead of
creating a new one. This made the code a tad more complex, and was also
a bit more annoying to use. This commit removes this extra piece of
information.
This means we now store much less information per function, that will
let us further optimise out structures in the future.
Now that we have recursive locks, the class creation code can be much simpler.
All the code there was essentially our own implementation of recursive locks,
or rather a special case of those.
This is no longer needed.
this adds a signle mutex (recursive) mutex for all eo objects that is
auto-called by _efl_object_call_resolve() and _efl_object_call_end()
that wrap all eo method calls and since its recursive it can be
blindly called for sub-calls. this will lock all shared objects during
any call to any shared object so only the thread calling now has
access until it releases. not fine-grained but good enough and the
best we can do "simplistically".
This moved all the eoid tables, eoid lookup caches, generation count
information ad eo_isa cache into a TLS segment of memory that is
thread private. There is also a shared domain for EO objects that all
threads can access, but it has an added cost of a lock. This means
objects accessed outside the thread they were created in cannot be
accessed by another thread unless they are adopted in temporarily, or
create4d with the shared domain active at the time of creation. child
objects will use their parent object domain if created with a parent
object passed in. If you were accessing EO (EFL) objects across threads
before then this will actually now cause your code to fail as it was
invalid before to do this as no actual objects were threadsafe in EFL,
so this will force things to "fail early".
ecore_thread_main_loop_begin() and end() still work as this uses the
eo domain adoption features to temporarily adopt a domain during this
section and then return it when done.
This returns speed back to eo brining the overhead in my tests of
lookup for the elm genlist autobounce test in elementary from about
5-7% down to 2.5-2.6%. A steep drop.
This does not mean everything is perfect. Still to do are:
1. Tests in the test suite
2. Some API's to help for sending objects from thread to thread
3. Make the eo call cache TLS data to make it also safe
4. Look at other locks in eo and probably move them to TLS data
5. Make eo resolve and call wrappers that call the real method func do
recursive mutex wrapping of the given object IF it is a shared object
to provide threadsafety transparently for shared objects (but adding
some overhead as a result)
6. Test test est, and that is why this commit is going in now for wider
testing
7. Decide how to make this work with sending IPC (between threads)
8. Deciding what makes an object sendable (a sendable property in base?)
9. Deciding what makes an object shareable (a sharable property in base?)
I knew Windows doesn't allow statically initialising pointers in the
global namespace, I had no idea it also applies to functions. That's
quite annoying.
Thanks to Cedric for reporting.
It has been discussed on the ML (thread: "[RFC] rename efl_self") and
IRC, and has been decided we should rename it to this in order to avoid
confusion with the already established meaning of self which is very
similar to what we were using it for, but didn't have complete overlap.
Kudos to Marcel Hollerbach for initiating the discussion and
fighting for it until he convinced a significant mass. :)
This commit breaks API, and depending on compiler potentially ABI.
@feature
As far as I remember, declaring structures and arrays in a cast is a GCC
extension. I'm not 100% sure I'm right, but I remember it was the case.
Regardless of whether it's an extension or not, this commit removes that
pattern and makes everything cleaner (and faster?).
This is another follow up to the investigations of T4227. As stated
there, in any PIE (a shared library is one), structures, even const ones
end up being written to because of dynamic relocation. This means that
using static const structures has actually lead to no savings, only
waste. Since we never really needed them, using them made things even
worse than just having a different API that doesn't save them.
Thus, this commit changes the way we set the functions. Instead of
passing a pre-populated struct, we now just have an initialiser function
where you set the functions. This on its own doesn't significantly reduce
the amount of dirty memory pages for a reason I have yet to uncover,
though I believe it's done as a misguided compiler optimisation.
However, this design is flexible enough so we can change to another one
that is quite ugly, but I have already tested and proven that does that.
This patch series doesn't include the better improvement (passing
everything on the stack as va_args) because the API was too ugly
for me to bear, and I would rather first make sure there is no way to
force the compiler to do the right thing here.
Unfortunately this commit gives up on useless stricter validation.
Before this commit we would make sure that we are only overriding
functions correctly defined in our hierarchy. With this one, we don't
anymore. This is not a big problem though because this is a check that
is also enforced by Eolian. So as long as you are using Eolian, you
should be fine.
Breaks API and ABI!
@feature
This improve speed of processing events in genlist scrolling benchmark by 30%
inside the efl_object_event_callback_call code. Not a really big deal as it
goes from 0.9% to 0.6% of the total time spend. Welcome to micro optimization.
also help reduce fragmentation. also remove callbacks immediately if
callbacks are not being walked at the time (as opposed to just marking
them to need deletion then call a clean that if not being walked will
walk all cb's when we already know what to remove).
@optimize
Efl.Object.event_callback_call no longer calls legacy smart callbacks;
calling only event callbacks registered with the given event description
pointer.
Create the method Efl.Object.event_callback_legacy_call to inherit the old
behavior from Efl.Object.event_callback_call, calling both Efl.Object events
and legacy smart callbacks.
Update all other files accordingly in order to still supply legacy
callbacks while they are necessary.
This was never used and there is no plan to ever use it. I'm going to
soon add a different mechanism with which it will be possible to provide
them again to Eo if ever needed without breaking ABI. Though it's
unlikely it will ever be.
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
So it may be used outside EO (eina error is what I have in mind).
I believe it doesn't need to be redefined in all EFL libs, especially
since it's not used on Windows yet.
Clang raised the warning:
redefinition of typedef 'Efl_Object' is a
C11 feature [-Wtypedef-redefinition]
for every compiling unit including Eo.h, which
caused a huge console pollution during compilation.
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
so after some discussion with jpeg, weak referenced keys are also a
good idea. these need del track handling to be weak, so i made strong
reffed keys also do del tracking again as it's just nice to do this
and be more robust. also added and improved the test suites for this
key value stuff.
@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 implements a new builtin, stringshare, which is replaced with the right
pointer to Eina_Strinshare as necessary. This allows simplifying binding code
(it can call the proper eina APIs, deal with lifetime etc).
It also removes the extern Eina.Stringshare typedef from eina_types.eot, which
was actually incorrect and would generate invalid code in binding generators.
@feature @fix
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.
Somehow, there was code in the tree that apparently isn't tested at all, even
once - if it was, the eo.c logic that performs inheritance checks would be
triggered. I don't know how this could have happened (actually I do, it's
Cedric's fault and he should be publicly shamed for it) but these checks
make sure this will never happen again. But since the code itself appears
to be untested, I don't know if there isn't any other brokenness in it.
But that's beyond the scope of this change, so for now, let's make sure
all our inheritance is at least formally correct.
Also, enable eo_interface.eo generated code in Eo itself so that Eo.Interface
can be used when inheriting.
@fix
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.
When compositing objects, we were checking that class_of(B) is in A's
inheritance tree before allowing attaching B as a composite object of A.
This is wrong and breaks a few cases, for example: B extends a class that
is in A's inheritance tree or B implements an interface that is in A's
inheritance tree.
Thanks to Marcel for reporting.
This lets me narrow down the remaining cases of pointers across the EFL.
The void pointers will later need to be reevaluated on per-case basis and
replaced appropriately where possible/feasible.
This is a new incarnation of 0a03e63350. Our list
has grown to big again as people insist of adding the generated eolian files to
DISTCLEAN while BUILT_SOURCES will get removed durign the clean anyway.
Adding this file list twice will just make the argument list for rm to long to
work.
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.
The legacy Eio_File factory functions are replaced by an Eo object
called Eo_Job that return promises wrapping the async file operations.
With this commit, the legacy Eio callbacks are replaced by the following
Eo/Promises counterparts :
* Done_Cb -> Promise then success callback
* Error_Cb -> Promise then error callback
* Main_Cb -> Promise progress callback
* Filter_Cb -> Job object event (more below)
Events are used to deliver and get the filter data. To differentiate
between the named and direct versions, they come in "filter,direct" and
"filter,name" versions.
Monitors were wrapped inside a new class Eo_Sentry.
The user creates a sentry object and adds monitoring targets to it,
listening to events on it.
The sentry event info is composed of two strings. The source string
is the path being monitored, i.e. the one passed to eio_sentry_add, and
the trigger string is the path that actually triggered the event, e.g.
a new file created in a monitored directory.
This problem was that because the refcount is now shared between the
parent and the programmer in some cases we would get a double unref. An
example way of triggering it is creating a button and putting it in a
box. The box has a callback registered that when the button is deleted
it would delete itself too. The problem is that the delete callback is
called the button is removed from the box thus causing the box to unref
it again (because of the parent), although the refcount was already
accounted for.
There is another more convoluted scenario that I have yet to fix.
Thanks to raster for reporting.
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.
I thought I compiled, but it seems that @q66 managed to distract
me and I thought wrong and didn't actually. Oh well, fixed now.
Thanks to @zmike for letting me know.
Complex types (i.e. list, array, hash, accessor etc.) now do not require
pointers with them anymore (the pointer is implied) and the same goes for
class handles. Eolian now explicitly disallows creating pointers to these
as well. This is the first part of the work to remove pointers from Eolian
completely, with the goal of simplifying the DSL (higher level) and therefore
making it easier for bindings (as well as easier API usage).
@feature
Apparently you can't cast when initializing static consts, even if
the cast is to the same type. This commit splits the macro used
so we have an additional one that casts and thus works with
eo_override().
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
Previously events used to use class name as a prefix and ignored eo_prefix
when specified. This is no longer the case. Events follow eo_prefix by default
now. In order to get around this for classes where this is undesirable, a new
field event_prefix was added which takes priority over eo_prefix. If neither
is specified, class name is used like previously.
@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.
In some build environment, anonymous char delcaration can be interpreted
as "unsigned char". Although lk_init can be only 0, 1 or 2 so there
won't be any unexpected result. This change is just to make static code
analyzer happy.
This commit breaks behaviour!
Re-parenting no longer detaches composite objects, so watch out.
Now you can have an object be a composite object of an object although
it's not its child. This allows widgets to do things like having an
object as the child of a child object while still making it a composite
object to the main object.
With this change, composite objects don't keep a reference to the child,
but instead composite "bonds" are implicitly removed when either the
parent or the child are destructed.
Summary:
object_find is more generic, so other mechanisms can also reuse the
code.
The object itself has to support the function, so there is no need for
eo_isa which would have a negative performance impact.
The base class implementation calls interface_get on the parent, so a
override of the function can just call the super function to continue in
the recursion.
Test Plan: just run the eo test suite
Reviewers: raster, tasn, jpeg
Reviewed By: tasn, jpeg
Subscribers: felipealmeida, netstar, cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D3909
Because we were forcing EWAPI in this macro, one couldn't create a class
that is "static" or even just private or the module. The symbol was
always exposed.
Since in C the attributes of a function are set based on the first
declaration, we don't need to specify any attributes in this macro and
we can just rely on them being specified in the declaration. So for
example, for class "foo":
foo.h:
EWAPI const Eo_Class *foo_class_get(...);
foo.c:
const Eo_Class *foo_class_get(...);
Would give the desired results, a class would be EWAPI. This is already
done automatically for all of the classes using Eolian. Because of the
lack of specifiers, the default visibility will now be the default
visibility based on compiler flags and settings.
Thanks to JP for reporting this issue.
This can potentially (but very unlikely) break things.
This code is an absolute mess. This is the first step towards fixing that.
This cleanup let me find a bug that would have printed errors when using
Eina_Value so I fixed that too.
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 geneirc data, wrefs, comments and id's are not that common so put
them all into their own memory segment that's allocated separately to
the core object so we only use this memory when needed. we already had
an extension section anyway so it's not new - just using it now for
more of the rarer bits of data. 2 more pointers gone from most objects
anyway and now extension code handling is a bit cleaner.
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).
base class objects will ask their parent object to get the loop. this
means it recurses all objects regardless of type until it finds an
object that returns a proper loop object - eg a loop object returns
itself, a window, gfx/ui object returns the mainloop object etc. etc.
@feature
The idea is that when you are processing those events from within
they won't call twice any of the event until all of them are processed
at least once (Or one of them did return EINA_FALSE).
this finds child objects by walking the child tree searching children
by using the search string which can be name, class:name and both
class and name can also be a glob.
@feature
eina value would allow any value to be attached to an eo object and
also be freed nicely too. this would allow any generic data to go
there without overloading a void * that us c coders love to abuse.
@feature
also useful for debugging and more. this also makes both name/id and
comment an extension blob of ram so objects dont keep growing
boundlessly in memory usage/size
@feature
also evas objects have names too, so add this as this was discussed
and now the feature is in. there is nothing to find objects by name
yet. that's more api's and features to add after this.
@feature
this adds eo_key_obj_set/get/del() like with data but for object
handles so the obj is ref'd as long as the key and parent obj exists
and then unreffed on deletion. it also tracks deletion of reffed
objects like weak refs and then removes the key automatically.
@feature
Modify the way hooks are defined and used by promise generation in
Eolian in the Eo API.
Instead of passing macro names as parameters to EO_FUNC_BODY macros,
just re-define the actual hooks when it is needed.
For windows, EAPI needs to be redefined as dllexport/dllimport.
Since we marked all EO APIs as weak, we had two different EAPI
macros: EAPI and EWAPI. Unfortunately, EWAPI was never redefined
(only declared inside Eo.h).
See also a1a506e13e.
See T3423. Thanks @vtorri for the report.
While eolian-gen was generating EWAPI (weak) class_get()
symbol declarations, they were implemented as EAPI (strong).
Not sure if this mismatch had any significant effect.
This patch tries to address T3423 (windows build).
The mismatch between EAPI and EWAPI might be the problem.
Add two parameters for macros that generate API functions in Eo so
that the generation can be customized with macros used by Eolian.
Signed-off-by: Cedric Bail <cedric@osg.samsung.com>
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.
The function call resolve cache may be broken after an eo
shutdown + init cycle, leading to calls to invalid functions.
This adds an static uint for each and every single EO API
entry point, as well as an extra if() check.
Now I'm not sure if the previous commit 0862b9d083 is
still necessary.
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.
This marks all EOAPI functions with GCC weak attribute.
This allows two things:
- replace functions
- link at runtime and check if functions exist
The purpose of this patch is to exploit these two features
of weak symbols. The first goal is to allow applications
to build binaries against later versions and run on earlier
versions of EFL without any run-time link error. Those errors
simply prevent applications to even start if they were using
any function that's not present in the old version of EFL.
Now all that needs to be done on the application side is to
do either of:
- if (efl_version > xx) { call_weak_symbol() }
- if (call_weak_symbol) { call_weak_symbol() }
In the future, we can also imagine providing a compatibility
library that would replace EFL's internal APIs with a newer
version. This would let apps use new EFLs on platforms that
don't update fast enough.
I am now pushing this patch as an experiment to see what breaks,
but I expect no problem.
@feature
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.
It was temporarily eoid, change it to eo_self which is more
descriptive. In this process I also made it a macro to prepare
for the proposed changes on the ML for the fallback implementation
for compilers that don't support the compound statements returning
values gcc extension.
I found a way to keep eo_add() the way it was and gracefully degrade to
a portable (but not as fast) solution for compilers that don't support
the compound macros returning a value gnu extension: ({int a; a;}).
I'm reverting these changes now, and I'll introduce the fallback as soon
as I can.
This reverts commit b85bb37183.
Reverting this at Felipe's request following my email. There are many
things I strongly object to in this commit. I've touched the surface of
those on the ML (which doesn't work at the moment), though we need to
better discuss it.
The gist:
1. dlsym is a really bad hack that is not even needed.
2. I don't see why eo should even be aware of promises. It's not aware
of list, hash and etc.
3. The eolian changes were done wrong.
This should have been discussed and consulted before done, even if only
because of the amount of hacks it includes and the cross-domain (ecore,
eo and eolian) nature of it.
This reverts commit f9ba80ab33.
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
Add a promise object that allows Eolian interface to include promises
as a way to have asynchronous value return and composibility.
The usage is like this in a .eo file:
class Foo {
methods {
bar {
params {
promise: Promise<int>;
}
}
}
}
Which will create the following API interface:
void foo_bar(Ecore_Promise** promise);
and the equivalent declaration for implementation.
However, the API function will instantiate the Promise for the
user and the implementer of the class.
Mostly unused vars following the removal of eo_do_ret().
However, there are some cases where the migration script got some things
wrong, and I had to manually fix them.
I just ran my script (email to follow) to migrate all of the EFL
automatically. This commit is *only* the automatic conversion, so it can
be easily reverted and re-run.
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
Change the Eo event callback signature to what suggested by Marcel
Hollerbach in the ML (Thread: EFL interface change - Animator).
This changes the signature of callbacks from
Eina_Bool cb(void *data, Eo *obj const Eo_Event_Description *desc, void *event_info)
to
Eina_Bool cb(void *data, const Eo_Event *event)
Where Eo_Event is a structure that holds these parameters.
This makes it less annoying to not use parameters (you end up using
EINA_UNUSED less), and allows for future extensions to callback
parameters.
@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.
Leave variables named Klass so it behaves with syntax highlighting and doesn't confuse programmers. However when Erroring, the message should be spelled correctly with "Class".
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).
Useful for GDB: break on this function when things go wrong.
Similar to eina_safety.
I guess we could set some Eina_Error and maybe even have error
callbacks for easier application debugging. Later.
Coverity CID1339783 says that we have a potential resource leak here.
'cb' gets allocated via calloc, but is not freed if we end up
returning here
@fix
Signed-off-by: Chris Michael <cpmichael@osg.samsung.com>
To configure efl sources with bindings to use in nodejs add ––with-js=nodejs in configure flags to generate node files
$ configure --with-js=nodejs
and compile normally with:
$ make
$ make install
To use, you have to require efl:
efl = require('efl')
The bindings is divided in two parts: generated and manually
written. The generation uses the Eolian library for parsing Eo files
and generate C++ code that is compiled against V8 interpreter library
to create a efl.node file that can be required in a node.js instance.
@feature
Gustavo Sverzut Barbieri
Bruno Dilly
Guilherme Iscaro
Carsten Haitzler
Marcel Hollerbach
Jean-Philippe Andre
Andy Williams
Cedric Bail
Tom Hacohen
Stefan Schmidt
Daniel Kolesa
Felipe Magno de Almeida
Jean Guyomarc'h
Vitor Sousa
Lauro Moura
Amitesh Singh
Davide Andreoli
Daniel Zaoui
Jaehyun Cho
Stephen Houston
Vincent Torri
Chris Michael