Some applications will create the handle, immediately send data, flush
and delete it, expecting the data to be sent to remote peer.
This is a bad behavior as the application would become unresponsive
until the connection is established, data can be written (since
depends on server consuming it), then allow it to be closed.
A proper behavior here would be to chain based on events, with the
usage of a copier would be simply wait for "done" event.
However the legacy API allowed this and terminology depends on this
awkward "feature", thus be bug-compatible.
This fixes T5015.
In the old/legacy API the socket would be opened early in non-blocking
mode (connect returned errno==EINPROGRESS), with UNIX socket being
path-validated early and returning NULL as 'server' handle.
Some applications relied on this instead of monitoring the "ERROR"
events, considering the connection to be successful if there was a
handle -- this was the case with Terminology after it moved from DBus
to Ecore_Ipc.
Although this is not correct, we must keep compatibility and thus we
stat() in compatibility layer, failing early as the old API would do.
These legacy API had the nasty behavior of keeping handles alive until
the pending events were dispatched, this could happen after the module
itself was shutdown, resulting in log to unregistered domains.
Then do not unregister the domain -- eina_shutdown will avoid leaks
anyway.
I always got this during the build:
lib/ecore_ipc/ecore_ipc.c:537:6: warning: ‘old_mask’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
Looking at the code it really is a false positive. Gettign the mask is behind
an if it is the same if condistion used before writing it. Anyway, silencing the
warning here.
The may_block parameter is useful to force a flush without blocking on
read/write, sometimes particularly useful if ignore_line_delimiter is
true, then you get the data events without blocking -- as if a server
sending some content misses a trailing line delimiter, you do not want
to block on recv() but still want to flush data to user.
The ignore_line_delimiter parameter is useful if we're going to close
the copier and want to flush pending data which may exist due missing
trailing terminator. The close method will also force that if
destination can take more data.
Each client (Ecore_Ipc_Client) is very similar to the handle
configured by ecore_ipc_server_connect() (the dialer), except we do
not have events such as "connected" and "error", as well as we don't
delete the socket as it's owned by the server, instead we close it.
The UNIX socket is configured similarly to ecore_con, setting the same
masks and mode for directories.
Use the new Efl.Net.Dialer classes to implement
ecore_con_server_connect() scenario.
Note that since Windows still doesn't provide any equivalent to
Efl.Net.Dialer.Unix, we keep the legacy code for it.
Using the ecore_ipc_server_example with -m/--single-message, if we
deleted the client from the callback it would find a dead "cl".
As the "cl" handle is removed from svr->clients before it's deleted,
it's safe to check if we have that handle in the list before
proceeding.
The flag 'delete_me' is set when there are pending events to be
dispatched. Once these events are freed, they will check if the server
was pending delete and call ecore_ipc_server_del() again, thus we must
not return, otherwise data will be leaked.
Summary: During mobile product testing, we got a crash with callstack which suggest server is getting deleted prior to client. On valgrind analysis we found invalid write operation with same callstack. callstack is pasted in comment section.
Test Plan: create a situation where server got deleted prior to client.
Reviewers: raster, cedric
Subscribers: govi, rajeshps, jpeg
Differential Revision: https://phab.enlightenment.org/D4152
Summary:
buf is always NULL (already freed and set to NULL).
We don't need to add NULL checking and free it.
Reviewers: raster, cedric, Hermet
Subscribers: seoz, cedric
Differential Revision: https://phab.enlightenment.org/D2783
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Summary: The allocated memory is not released before return.
Lost track of the CID.
Test Plan: Run static analysis tool such as prevent
Reviewers: raster, cedric
Reviewed By: cedric
Subscribers: cedric, seoz
Differential Revision: https://phab.enlightenment.org/D1746
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Being annoyed by different types of eina critical macros - CRI, CRIT,
CRITICAL -, I concluded to unify them to one. Discussed on IRC and
finally, CRI was chosen to meet the consistency with other macros -
ERR, WRN, INF, DBG - in terms of the number of characters.
If there is any missing bits, please let me know.
According to clang static analyzer it is possible to find a path where
buf and svr->buf are pointing to the same array, better be safe than sorry.
Arguably this code could be more readable if it was using Eina_Binbuf.
freeing first) leaks the storage that buf Did point to (which could
have been from a realloc above).
NB: Fixes Coverity CID1039277, CID1039278
Signed-off-by: Chris Michael <cp.michael@samsung.com>
Gustavo Sverzut Barbieri
Stefan Schmidt
Jee-Yong Um
Prince Kumar Dubey
Amitesh Singh
Thiep Ha
Cedric BAIL
Shinwoo Kim
Pierre Le Magourou
Jihoon Kim
Daniel Juyung Seo
Cedric Bail
Carsten Haitzler
Chris Michael
Chris Michael
Cedric Bail
Lucas De Marchi
Gustavo Sverzut Barbieri
Mike Blumenkrantz
Jonas M. Gastal
Vincent Torri