i found some more cases where the hash may change, so del and add from
the hash when this happens...
and... i found a nasty. _edje_signal_match_key_cmp compared pointers
like:
int return = ptr_a - ptr_b;
what happens if .... ptr_a and ptr_b are more than 2^31 (2gb) apart?
overflow (or underflow) and we return the wrong thing. i suspect this
is part of the problem and why my has remove/adds have not been
working because ... i suspect that maybe the hash dels have not been
finding things. i can't be sure right now, but it is an obvious
problem that i fixed by just doing if's and returning -1 or 1. also i
found a double-add or overwrite int he hash - when we shuffled with
_edje_signal_callback_move_last the matches CAN match exactly
something already in the hash thus adding it in will conflict with
what is already there as keys match. handle this cvase now and i have
seen segv's go away for now.
@fix
so i think there was also another bug as i saw this again now. this
should hopefully plug the signal/mtach memory bugs now by removing
and adding back to hash as the hash key relies on the memory addresses
of signal/src in the matches array which change on realloc. a bit
brute-forceey but... better than crashes.
@fix
also a general cleanup of the code to make it easier to follow/read as
this seems to have problems but i cant reproduce them enough to find
them. i noted a realloc would have invalidated pre-stored pattern
ptrs that would cause segv's for sure. also code paths where reallocs may
fail and not handling those cases at all etc.
@fix
Summary:
- when deleted callback is found _edje_signal_callback_move_last() is called
in order to pack match array.
- during _edje_signal_callback_move_last() index skips when another deleted
callback is found, but does not reduce members_count.
- this duplicates a remaining callback and calls the callback twice.
Test Plan:
1. add multiple edje_signal_callback by edje_object_signal_callback_add()
which have the same source, signal, func but different data.
2. delete first and last callback by
edje_object_signal_callback_del/edje_object_signal_callback_del_full.
3. emit edje_signal.
4. observe one callback is called twice.
Reviewers: SanghyeonLee, conr2d, jpeg
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4985
let's assume you registered a callback twice with the same ptr data
and same func and same sig/src... if you delete it once you're ok.
delete twice... and you re-delete the first one (just makging it for
delete_me). this fixes this corner case
@fix
i just got a segv freeing ian invalid ptr... it SEEMS as if something
has tried to free and edje object twice... but i can't be sure as
valgrind can't catch this. it's a one-off, so ensure after free, we
NULL out things we freed to avoid this.
@fix
Big thanks to Maxim who did a serious digging in this issue making it almost
a patch review.
T3125
@fix
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
this just clens up the _edje_signal_callback_push() to be simpler and
less wordy with the same actual logic, just pointless things like
return; at end of func removed, use tmp instead of gp->matches
everywhere and not just in one section etc.
also set hashed bool to eina true/false i as opposed to sometimes 0,
sometimes eina true/false and also track it religiously as well as
matches array when freed - hunting bug
for whatever reason after these cleanups i can't reproduce a signal
crash i had which seemed to find freed matches in the hash that should
not have been there. (a hash find walking a bucket found freed memory
for the match in the hash entry - should not have been though reading
the code).
@fix
ok. i found this once-ever-seen thing where
Edje_Signal_Callback_Matches has ALL fields NULL/0 except refcount was
huge (like 13834275 or something like that). i can't see why at the
moment, but defend against it to avoid crashes here by handling these
being null
Share a byte with 2 matchs and use bitwise operations to read/write they states
is not much readable and easily could lead a issues.
Use a struct is much simpler and only will coast a half of byte per match.
This was causing some callback be removed instead a deleted callback.
Also was leaking stringshare and decreasing matches_count.
SPANK SPANK SPANK cedric
This patch try to share signal callbacks and automate accross all Edje
object. It does use an Eina_Hash on the callback description (signal,
source, func). There is no need to check it against Edje file or group
only the callbacks matter.
This version remove all use of size_t as it should never be above 32bits.
We have a hard limit on the number of callback to 2^32 now. I am considering
it would be sane to make it a short.
Program received signal SIGSEGV, Segmentation fault.
_edje_emit_cb (ed=ed@entry=0x8ebd68, sig=sig@entry=0x8fba4c
"focus,part,in", src=src@entry=0x8222fc "elm.text",
data=data@entry=0x0, prop=prop@entry=0 '\000') at
lib/edje/edje_program.c:1442
warning: Source file is more recent than executable.
1442 eina_list_free(data.matches);
(gdb) bt
"focus,part,in", src=src@entry=0x8222fc "elm.text",
data=data@entry=0x0, prop=prop@entry=0 '\000') at
lib/edje/edje_program.c:1442
"focus,part,in", src=0x8222fc "elm.text", sdata=0x0, prop=0 '\000') at
lib/edje/edje_program.c:1366
_pd=0x8222f0, list=0x7fff00000001) at lib/edje/edje_message_queue.c:124
op_type=EO_OP_TYPE_REGULAR, op=564, p_list=0x7fffffffcf28) at
lib/eo/eo.c:364
op_type=EO_OP_TYPE_REGULAR, obj=0x8ebae0) at lib/eo/eo.c:404
op_type=op_type@entry=EO_OP_TYPE_REGULAR) at lib/eo/eo.c:435
(obj=<optimized out>) at lib/edje/edje_message_queue.c:71
(sid=sid@entry=0x8eae88) at elm_interface_scrollable.c:788
(sid=0x8eae88) at elm_interface_scrollable.c:805
_pd=0x8eae88, list=<optimized out>) at elm_interface_scrollable.c:3370
op_type=EO_OP_TYPE_REGULAR, op=840, p_list=0x7fffffffd158) at
lib/eo/eo.c:364
op_type=EO_OP_TYPE_REGULAR, obj=0x8ea9e0) at lib/eo/eo.c:404
op_type=op_type@entry=EO_OP_TYPE_REGULAR) at lib/eo/eo.c:435
_pd=0x8ead70, list=<optimized out>) at elm_entry.c:2804
op_type=EO_OP_TYPE_REGULAR, op=129, p_list=0x7fffffffd328) at
lib/eo/eo.c:364
op_type=EO_OP_TYPE_REGULAR, obj=0x8ea9e0) at lib/eo/eo.c:404
lib/eo/eo.c:435
op_type=EO_OP_TYPE_REGULAR, op=1, p_list=0x7fffffffd4a8) at
lib/eo/eo.c:364
(obj=obj@entry=0x8ea9e0, op_type=op_type@entry=EO_OP_TYPE_REGULAR,
op=1) at lib/eo/eo.c:463
class_data=<optimized out>, list=<optimized out>) at elm_widget.c:5526
op_type=EO_OP_TYPE_REGULAR, op=1, p_list=0x7fffffffd628) at
lib/eo/eo.c:364
(obj=obj@entry=0x8ea9e0, op_type=op_type@entry=EO_OP_TYPE_REGULAR,
op=1) at lib/eo/eo.c:463
out>, list=<optimized out>) at elm_layout.c:2157
op_type=EO_OP_TYPE_REGULAR, op=1, p_list=0x7fffffffd7b8) at
lib/eo/eo.c:364
op_type=<optimized out>, op=1) at lib/eo/eo.c:463
op_type=EO_OP_TYPE_REGULAR, op=1, p_list=0x7fffffffd928) at
lib/eo/eo.c:364
(obj=obj@entry=0x8ea9e0, op_type=op_type@entry=EO_OP_TYPE_REGULAR,
op=1) at lib/eo/eo.c:463
out>, list=<optimized out>) at elm_entry.c:3076
op_type=EO_OP_TYPE_REGULAR, op=1, p_list=0x7fffffffdad8) at
lib/eo/eo.c:364
op_type=EO_OP_TYPE_REGULAR, obj=0x8ea9e0) at lib/eo/eo.c:404
lib/eo/eo.c:1162
elm_entry.c:3068
autorun=0x0) at test.c:441
Revert "edje: reduce memory usage of Edje signal callbacks and automates."
This reverts commit 15aae2c0a4.
This patch try to share signal callbacks and automate accross all Edje
object. It does use an Eina_Hash on the callback description (signal,
source, func). There is no need to check it against Edje file or group
only the callbacks matter.