Summary:
Coverity reports that 'svr' might be NULL here, so we should check it
is valid before trying to use it.
Fixes Coverity CID1396990
@fix
Depends on D8320
Reviewers: raster, cedric, q66, zmike, bu5hm4n, stefan
Reviewed By: cedric
Subscribers: #reviewers, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D8321
non-thread safe functions are used like rand(), strerror().
this patch replace them with thread safe one.
and also this patch contains a change to fix a memory leak problem.
Reviewed-by: Cedric BAIL <cedric.bail@free.fr>
Differential Revision: https://phab.enlightenment.org/D7917
This has been the fix that prevent random ecore_con DNS tests to fail for me. This should also in case
of failure make the timeout of the ecore_con itself be honored which wasn't anymore.
@fix
Differential Revision: https://phab.enlightenment.org/D7402
Summary: for hard failures (highly unlikely), the handle would be deleted and the next use would crash.
Test Plan: N/A
Reviewers: Jaehyun_Cho, zmike, cedric, bu5hm4n
Reviewed By: bu5hm4n
Subscribers: bu5hm4n, #reviewers, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D7139
Summary:
in the course of the efl-net rewrite, the previous relied-upon behavior
sequence of error -> disconnect -> del was broken, and error events no
longer triggered disconnects or server deletion.
the failure was caused by a check for the 'connecting' flag, preventing
DEL events from being triggered
further, the ordering of the events was flipped from ERROR -> DEL to
DEL -> ERROR; this is also fixed now
this caused any failed connection to leak the entire server connection
since existing handlers for the del event were never triggered and the
server was never destroyed
@fix
fix T6330
Reviewers: devilhorns
Reviewed By: devilhorns
Subscribers: cedric, #committers
Tags: #efl
Maniphest Tasks: T6330
Differential Revision: https://phab.enlightenment.org/D6481
Summary:
"cl->svr" was to be null on line 281.
This problem occurred from 384f30c8ec.
Replacing "cl->svr" with "svr" corrects the error
while preserving the changes in 384f30c8ec.
Test Plan: N/A
Reviewers: #committers, cedric, Hermet, zmike
Subscribers: bowonryu, zmike
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6255
Summary:
cl->socket is a Efl.Net.Socket_Simple. it is not inherit the Efl.Loop.fd class.
and The target of the class check have to be the inner_socket.
But inner_socket is Efl.Net.Socket_Ssl. Efl.Net.Socket_Ssl class is not inherit FD class.
Efl.Net.Socket_Tcp is inherit Efl.Loop.fd class. So, Need to add Efl.Net.Socket_Tcp to inheritance.
(The server side is a similar hierarchy. (ssl -> tcp -> ip -> fd))
Test Plan: N/A
Reviewers: cedric, jypark, myoungwoon, zmike, barbieri
Reviewed By: cedric
Subscribers: #committers, bowonryu, zmike
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6168
Reviewed-by: Cedric BAIL <cedric@osg.samsung.com>
Ecore_Event can still be in flight while the client get freed by the server.
We are protected agaist that and keep the client partially alive until the event
are processed, but we need to disconnect from the server as soon as possible
as we might outlive the server.
Summary:
space("") name check had existed, but it was removed on f4306d654d
(in the _efl_network_connector_efl_object_finalize)
This patch restore the safety checks
Reviewers: barbieri, Hermet, Jaehyun_Cho, myoungwoon, zmike
Reviewed By: zmike
Subscribers: cedric, zmike
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6177
This reverts commit 135154303b.
Revert "efl: move signal events from efl.loop to efl.app"
This reverts commit 3dbca39f98.
Revert "efl: add test suite for efl_app"
This reverts commit 3e94be5d73.
Revert "efl: create Efl.App class, the parent of Efl.Loop"
This reverts commit 28fe00b94e.
Go back to before efl.app because I think this should be done with
superclassing here not a parent object. reasons?
1. multiple loops per single thread make no sense. so if multilpe loop
objects they wont be contained in a single app object and then deleted
like this.
2. the app object is not really sharable in this design so it cant be
accessed from other threads
3. it makes it harder to get the main loop or app object (well 2 func
calls one calling the other and more typing. it is longer to type and
more work where it is not necessary, and again it can't work from
other threads unless we go duplicating efl.app per thread and then
what is the point of splittyign out the signal events from efl.loop
then?)
etc.
Summary:
This patch checks whether the port number is valid or not.
The valid port number is an unsigned 16-bit integer, so 1-65535.
0 is reserved already.
Test Plan: Execute test suite
Reviewers: cedric, raster, stefan, Jaehyun_Cho
Reviewed By: raster
Subscribers: jpeg
Differential Revision: https://phab.enlightenment.org/D5761
by deleting the socket they wont mutually exclude at bind which means
eny new server just nides the pror one and you can get lots of copies
of the same server. this wasnt the case before. it shouldnt have been.
i think ther was an rm path for stale sockets when connecting failed
or something. anyway... this here was causing multilpe efreetd's and
all sorts of nastiness. this is the root cause. so... fix it.
@fix
Summary:
The _ecore_con_post_event_server_upgrade() call adds an event to free
the server_upgrade object, svr, via _ecore_con_server_free(svr) so we
should assume srv is freed after it returns. Thus, perform the
pending_slice processing prior to calling it. Otherwise it triggers an
illegal access (USE_AFTER_FREE) error in Coverity.
@fix CID1373485
Reviewers: barbieri
Reviewed By: barbieri
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4785
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Since efl_net_ssl_context is immutable for a dialer and we create the
dialer with the context, the SSL cases uses a trick to postpone dialer
creation using a job, then it allows one main loop iteration for the
user to call various ecore_con_server_ssl_*() methods.
However this breaks immediate ecore_con_server_send() after
ecore_con_server_connect() as used to be allowed and used by
azy/erssd.
Most people wouldn't notice that, since the most common case is to
either use ecore_con_url (which uses cURL and a complete different
code path) OR they would wait for ECORE_CON_EVENT_SERVER_ADD prior to
sending data.
Nonetheless it was a compatibility issue and must be fixed.
Fixes T5339
This is the local socket for windows, analogous to AF_UNIX.
`Efl_Net_Socket_Windows` is the base class doing `ReadFile()` and
`WriteFile()` using overlapped I/O, as well as the close procedure
(`FlushFileBuffers()`, `DisconnectNamedPipe()` and
`CloseHandle()`). These are done on top of an existing HANDLE that is
set by `Efl_Net_Dialer_Windows` (from `CreateFile()`) or
`Efl_Net_Server_Windows` (from `CreateNamedPipe()`).
The overlapped I/O will return immediately, either with operation
completed or `ERROR_IO_PENDING`, which means the kernel will execute
that asynchronously and will later `SetEvent(overlapped.hEvent)` which
is an event we wait on our main loop. That `overlapped` handle must
exist during the call lifetime, thus cannot be bound to `pd`, as we
may call `CancelIo()` but there is no guarantee the memory won't be
touched, in that case we keep the overlapped around, but without an
associated object.
Windows provides no notification "can read without blocking" or
non-blocking calls that returns partial data. The way to go is to use
these overlapped I/O, with an initial `ReadFile()` to an internal
buffer, once that operation finishes, we callback the user to says
there is something to read (`efl_io_reader_can_read_set()`) and wait
until `efl_io_reader_read()` is called to consume the available data,
then `ReadFile()` is called again to read more data to the same
internal buffer.
Likewise, there is no "can write without blocking" or non-blocking
calls that sends only partial data. The way to go is to get user bytes
in `efl_io_writer_write()` and copy them in an internal buffer, then
call `WriteFile()` on that and inform the user nothing else can be
written until that operation completes
(`efl_io_writer_can_write_set()`).
This is cumbersome since we say we "sent" stuff when we actually
didn't, it's still in our internal buffer (`pd->send.bytes`), but
nonetheless the kernel and the other peer may be adding even more
buffers, in this case we need to do a best effort to get it
delivery. A particular case is troublesome: `write() -> close()`, this
may result in `WriteFile()` pending, in this case we wait using
`GetOverlappedResult()`, *this is nasty and may block*, but it's the
only way I see to cope with such common use case.
Other operations, like ongoing `ReadFile()` or `ConnectNamedPipe()`
will be canceled using `CancelIo()`.
Q: Why no I/O Completion Port (IOCP) was used? Why no
CreateThreadpoolIo()? These perform much better!
A: These will call back from secondary threads, but in EFL we must
report back to the user in order to process incoming data or get
more data to send. That is, we serialize everything to the main
thread, making it impossible to use the benefits of IOCP and
similar such as CreateThreadpoolIo(). Since we'd need to wakeup the
main thread anyways, using `OVERLAPPED.hEvent` with
`ecore_main_win32_handler_add()` does the job as we expect.
Thanks to Vincent Torri (vtorri) for his help getting this code done
with an example on how to do the NamedPipe handling on Windows.
While a socket can be closed to receive data resulting in EOS, it
could still be used to send stuff. Then it won't result in "finished",
just "read,finished" event.
However, previously this was considered a disconnect and we must
respect this otherwise tests (Ecore_Con_Eet suite) will hang waiting
for a disconnect.
Previously we couldn't return a slice, instead required the user to
pass a slice and we'd fill it since Eolian couldn't generate fallbacks
for structures.
Since @q66 fixed eolian, we can now return the structure itself as
initially wanted, ditching some TODO from the code.
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.
This is a major work and unfortunately couldn't be split into smaller
pieces as old code was highly coupled.
Ecore_Con_Server is now a wrapper around Efl_Net_Dialer_Simple
(ecore_con_server_connect()) and Efl_Net_Server_Simple
(ecore_con_server_add()), doing all that the original version did with
some fixes so ecore_con_ssl_server_upgrade() and
ecore_con_ssl_client_upgrade() are more usable -- see the examples and
-t/--type=tcp+ssl.
I tried to be bug-compatible, with code annotations where things
doesn't make sense. This was based on ecore_con_suite tests and some
manual experimenting with the examples, these can be helpful if you
find regressions (report/assign to me).
This removes some useless code in various places, where the
switch from eo_do() to standard function call was not properly
refactored.
This changes:
type ret = 0;
ret = my_eo_function();
return ret;
To:
return my_eo_function();
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.
Summary: This is just the beginning. I tried for one class to check.
Tell me if this is fine, I'll change in other classes also. The goal
is to simplify and make our API clearer to understand to new comers.
Reviewers: cedric
Subscribers: cedric
Differential Revision: https://phab.enlightenment.org/D2468
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
Vincent Torri
Christopher Michael
Mike Blumenkrantz
Wonki Kim
Cedric BAIL
Carsten Haitzler
junsu choi
Alastair Poole
Cedric Bail
YeongJong Lee
Mike Blumenkrantz
Myoungwoon Roy, Kim
Guilherme Iscaro
Bryce Harrington
Gustavo Sverzut Barbieri
Jean-Philippe Andre
Tom Hacohen
Srivardhan Hebbar