barbieri pushed a commit to branch master.
http://git.enlightenment.org/core/efl.git/commit/?id=410d65900c9ce4daa412047c8b49312b98a1d353
commit 410d65900c9ce4daa412047c8b49312b98a1d353
Author: Gustavo Sverzut Barbieri <barbi...@profusion.mobi>
Date: Fri Nov 25 05:32:16 2016 -0200
efl_net_server: add 'client_announce', share logic and fix a bug.
I just realized that if a client is not referenced it would leak in
the 'ssl' server as we must del it.
However, if we del the SSL socket, we're going to close the underlying
TCP. But we're from the TCP "client,add" callback and this causes
issues since "closed" will be emitted, our close callback will
unparent the client, which lead to it being deleted.
The proper solution is to only monitor "closed" if the client is
accepted. Otherwise we just check if it was closed, if we're the
parent, etc...
Fixing this in all servers were painful, we could share since most
inherit from Efl.Net.Server.Fd. Then add the "client_announce"
protected method to do it, and document how it should work.
---
src/lib/ecore_con/efl_net_server.eo | 39 ++++++++++++++++++++
src/lib/ecore_con/efl_net_server_fd.c | 61 +++++++++++++++++++++++++++++++
src/lib/ecore_con/efl_net_server_fd.eo | 1 +
src/lib/ecore_con/efl_net_server_ssl.c | 63 ++++++++++++++++++++++++++++++++-
src/lib/ecore_con/efl_net_server_ssl.eo | 1 +
src/lib/ecore_con/efl_net_server_tcp.c | 29 +--------------
src/lib/ecore_con/efl_net_server_udp.c | 26 +++-----------
src/lib/ecore_con/efl_net_server_unix.c | 29 +--------------
8 files changed, 170 insertions(+), 79 deletions(-)
diff --git a/src/lib/ecore_con/efl_net_server.eo
b/src/lib/ecore_con/efl_net_server.eo
index 00084ea..f4a7cd7 100644
--- a/src/lib/ecore_con/efl_net_server.eo
+++ b/src/lib/ecore_con/efl_net_server.eo
@@ -99,6 +99,45 @@ interface Efl.Net.Server {
}
}
+ client_announce @protected {
+ [[Implementions should call this method to announce new clients.
+
+ This method will account the new client in
+ @.clients_count as well as emit the event "client,add".
+
+ After this call, the client ownership will be
+ managed. If no event handler references the object, it
+ will be deleted.
+
+ Most implementions will do the sequence:
+
+ - emit "client,add"
+
+ - check if client was referenced
+
+ - if we're not the parent anymore, ignore (do not
+ change @.clients_count) and return $true.
+
+ - if not referenced, delete it and return $false.
+
+ - if it's closed, delete it and return $false.
+
+ - if referenced, increment @.clients_count and monitor
+ for client "closed" event and return $true.
+
+ - on client "closed", decrease @.clients_count and
+ unset its parent (if we're still the parent).
+
+ Do not monitor "closed" before emitting
+ "client,add". Doing so may lead to double free if
+ callbacks close the client by themselves!
+ ]]
+ params {
+ client: Efl.Net.Socket; [[A socket representing the client.]]
+ }
+ return: bool(false); [[If $true, then client was handled. If
$false, it was dropped and deleted.]]
+ }
+
@property serving {
[[Returns whenever the server is ready to accept clients or not.
diff --git a/src/lib/ecore_con/efl_net_server_fd.c
b/src/lib/ecore_con/efl_net_server_fd.c
index b10a260..8506c6e 100644
--- a/src/lib/ecore_con/efl_net_server_fd.c
+++ b/src/lib/ecore_con/efl_net_server_fd.c
@@ -487,4 +487,65 @@ _efl_net_server_fd_process_incoming_data(Eo *o,
Efl_Net_Server_Fd_Data *pd)
efl_net_server_fd_client_add(o, client);
}
+static void
+_efl_net_server_fd_client_event_closed(void *data, const Efl_Event *event)
+{
+ Eo *server = data;
+ Eo *client = event->object;
+
+ efl_event_callback_del(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_fd_client_event_closed, server);
+ if (efl_parent_get(client) == server)
+ efl_parent_set(client, NULL);
+
+ efl_net_server_clients_count_set(server,
efl_net_server_clients_count_get(server) - 1);
+}
+
+static Eina_Bool
+_efl_net_server_fd_efl_net_server_client_announce(Eo *o,
Efl_Net_Server_Fd_Data *pd EINA_UNUSED, Eo *client)
+{
+ EINA_SAFETY_ON_NULL_RETURN_VAL(client, EINA_FALSE);
+ EINA_SAFETY_ON_FALSE_GOTO(efl_isa(client, EFL_NET_SOCKET_INTERFACE),
wrong_type);
+ EINA_SAFETY_ON_FALSE_GOTO(efl_parent_get(client) == o, wrong_parent);
+
+ efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client);
+
+ if (efl_parent_get(client) != o)
+ {
+ DBG("client %s was reparented! Ignoring it...",
+ efl_net_socket_address_remote_get(client));
+ return EINA_TRUE;
+ }
+
+ if (efl_ref_get(client) == 1) /* users must take a reference themselves */
+ {
+ DBG("client %s was not handled, closing it...",
+ efl_net_socket_address_remote_get(client));
+ efl_del(client);
+ return EINA_FALSE;
+ }
+ else if (efl_io_closer_closed_get(client))
+ {
+ DBG("client %s was closed from 'client,add', delete it...",
+ efl_net_socket_address_remote_get(client));
+ efl_del(client);
+ return EINA_FALSE;
+ }
+
+ efl_net_server_clients_count_set(o, efl_net_server_clients_count_get(o) +
1);
+ efl_event_callback_add(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_fd_client_event_closed, o);
+ return EINA_TRUE;
+
+ wrong_type:
+ ERR("%p client %p (%s) doesn't implement Efl.Net.Socket interface, deleting
it.", o, client, efl_class_name_get(efl_class_get(client)));
+ efl_io_closer_close(client);
+ efl_del(client);
+ return EINA_FALSE;
+
+ wrong_parent:
+ ERR("%p client %p (%s) parent=%p is not our child, deleting it.", o,
client, efl_class_name_get(efl_class_get(client)), efl_parent_get(client));
+ efl_io_closer_close(client);
+ efl_del(client);
+ return EINA_FALSE;
+}
+
#include "efl_net_server_fd.eo.c"
diff --git a/src/lib/ecore_con/efl_net_server_fd.eo
b/src/lib/ecore_con/efl_net_server_fd.eo
index dbafe1b..10fe712 100644
--- a/src/lib/ecore_con/efl_net_server_fd.eo
+++ b/src/lib/ecore_con/efl_net_server_fd.eo
@@ -161,5 +161,6 @@ class Efl.Net.Server.Fd (Efl.Loop.Fd, Efl.Net.Server) {
Efl.Net.Server.clients_limit;
Efl.Net.Server.serving;
Efl.Net.Server.serve;
+ Efl.Net.Server.client_announce;
}
}
diff --git a/src/lib/ecore_con/efl_net_server_ssl.c
b/src/lib/ecore_con/efl_net_server_ssl.c
index 4fa44eb..91810ea 100644
--- a/src/lib/ecore_con/efl_net_server_ssl.c
+++ b/src/lib/ecore_con/efl_net_server_ssl.c
@@ -20,6 +20,67 @@ typedef struct _Efl_Net_Server_Ssl_Data
} Efl_Net_Server_Ssl_Data;
static void
+_efl_net_server_ssl_client_event_closed(void *data, const Efl_Event *event)
+{
+ Eo *server = data;
+ Eo *client = event->object;
+
+ efl_event_callback_del(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_ssl_client_event_closed, server);
+ if (efl_parent_get(client) == server)
+ efl_parent_set(client, NULL);
+
+ /* do NOT change count as we're using the underlying server's count */
+ //efl_net_server_clients_count_set(server,
efl_net_server_clients_count_get(server) - 1);
+}
+
+static Eina_Bool
+_efl_net_server_ssl_efl_net_server_client_announce(Eo *o,
Efl_Net_Server_Ssl_Data *pd EINA_UNUSED, Eo *client)
+{
+ EINA_SAFETY_ON_NULL_RETURN_VAL(client, EINA_FALSE);
+ EINA_SAFETY_ON_FALSE_GOTO(efl_isa(client, EFL_NET_SOCKET_SSL_CLASS),
wrong_type);
+ EINA_SAFETY_ON_FALSE_GOTO(efl_parent_get(client) == o, wrong_parent);
+
+ efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client);
+
+ if (efl_parent_get(client) != o)
+ {
+ DBG("client %s was reparented! Ignoring it...",
+ efl_net_socket_address_remote_get(client));
+ return EINA_TRUE;
+ }
+
+ if (efl_ref_get(client) == 1) /* users must take a reference themselves */
+ {
+ DBG("client %s was not handled, closing it...",
+ efl_net_socket_address_remote_get(client));
+ efl_del(client);
+ return EINA_FALSE;
+ }
+ else if (efl_io_closer_closed_get(client))
+ {
+ DBG("client %s was closed from 'client,add', delete it...",
+ efl_net_socket_address_remote_get(client));
+ efl_del(client);
+ return EINA_FALSE;
+ }
+
+ /* do NOT change count as we're using the underlying server's count */
+ //efl_net_server_clients_count_set(o, efl_net_server_clients_count_get(o) +
1);
+ efl_event_callback_add(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_ssl_client_event_closed, o);
+ return EINA_TRUE;
+
+ wrong_type:
+ ERR("%p client %p (%s) doesn't implement Efl.Net.Socket.Ssl class, deleting
it.", o, client, efl_class_name_get(efl_class_get(client)));
+ efl_del(client);
+ return EINA_FALSE;
+
+ wrong_parent:
+ ERR("%p client %p (%s) parent=%p is not our child, deleting it.", o,
client, efl_class_name_get(efl_class_get(client)), efl_parent_get(client));
+ efl_del(client);
+ return EINA_FALSE;
+}
+
+static void
_efl_net_server_ssl_tcp_client_add(void *data, const Efl_Event *event)
{
Eo *o = data;
@@ -44,7 +105,7 @@ _efl_net_server_ssl_tcp_client_add(void *data, const
Efl_Event *event)
return;
}
- efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client_ssl);
+ efl_net_server_client_announce(o, client_ssl);
}
static void
diff --git a/src/lib/ecore_con/efl_net_server_ssl.eo
b/src/lib/ecore_con/efl_net_server_ssl.eo
index 27f3f79..0caadb8 100644
--- a/src/lib/ecore_con/efl_net_server_ssl.eo
+++ b/src/lib/ecore_con/efl_net_server_ssl.eo
@@ -138,6 +138,7 @@ class Efl.Net.Server.Ssl (Efl.Loop_User, Efl.Net.Server) {
Efl.Object.constructor;
Efl.Object.destructor;
Efl.Net.Server.serve;
+ Efl.Net.Server.client_announce;
Efl.Net.Server.address.get;
Efl.Net.Server.clients_count.get;
Efl.Net.Server.clients_limit;
diff --git a/src/lib/ecore_con/efl_net_server_tcp.c
b/src/lib/ecore_con/efl_net_server_tcp.c
index 03afd89..bdb0812 100644
--- a/src/lib/ecore_con/efl_net_server_tcp.c
+++ b/src/lib/ecore_con/efl_net_server_tcp.c
@@ -242,29 +242,10 @@ _efl_net_server_tcp_efl_net_server_serve(Eo *o,
Efl_Net_Server_Tcp_Data *pd, con
return 0;
}
-static Efl_Callback_Array_Item *_efl_net_server_tcp_client_cbs(void);
-
-static void
-_efl_net_server_tcp_client_event_closed(void *data, const Efl_Event *event)
-{
- Eo *server = data;
- Eo *client = event->object;
-
- efl_event_callback_array_del(client, _efl_net_server_tcp_client_cbs(),
server);
- if (efl_parent_get(client) == server)
- efl_parent_set(client, NULL);
-
- efl_net_server_clients_count_set(server,
efl_net_server_clients_count_get(server) - 1);
-}
-
-EFL_CALLBACKS_ARRAY_DEFINE(_efl_net_server_tcp_client_cbs,
- { EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_tcp_client_event_closed });
-
static void
_efl_net_server_tcp_efl_net_server_fd_client_add(Eo *o,
Efl_Net_Server_Tcp_Data *pd EINA_UNUSED, int client_fd)
{
Eo *client = efl_add(EFL_NET_SOCKET_TCP_CLASS, o,
- efl_event_callback_array_add(efl_added,
_efl_net_server_tcp_client_cbs(), o),
efl_io_closer_close_on_exec_set(efl_added,
efl_net_server_fd_close_on_exec_get(o)),
efl_io_closer_close_on_destructor_set(efl_added,
EINA_TRUE),
efl_loop_fd_set(efl_added, client_fd));
@@ -275,15 +256,7 @@ _efl_net_server_tcp_efl_net_server_fd_client_add(Eo *o,
Efl_Net_Server_Tcp_Data
return;
}
- efl_net_server_clients_count_set(o, efl_net_server_clients_count_get(o) +
1);
- efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client);
-
- if (efl_ref_get(client) == 1) /* users must take a reference themselves */
- {
- DBG("client %s was not handled, closing it...",
- efl_net_socket_address_remote_get(client));
- efl_del(client);
- }
+ efl_net_server_client_announce(o, client);
}
static void
diff --git a/src/lib/ecore_con/efl_net_server_udp.c
b/src/lib/ecore_con/efl_net_server_udp.c
index 6e38a8f..4b29793 100644
--- a/src/lib/ecore_con/efl_net_server_udp.c
+++ b/src/lib/ecore_con/efl_net_server_udp.c
@@ -278,8 +278,6 @@ _efl_net_server_udp_efl_net_server_serve(Eo *o,
Efl_Net_Server_Udp_Data *pd, con
return 0;
}
-static Efl_Callback_Array_Item *_efl_net_server_udp_client_cbs(void);
-
static void
_efl_net_server_udp_client_event_closed(void *data, const Efl_Event *event)
{
@@ -287,17 +285,10 @@ _efl_net_server_udp_client_event_closed(void *data, const
Efl_Event *event)
Eo *client = event->object;
Efl_Net_Server_Udp_Data *pd = efl_data_scope_get(server, MY_CLASS);
- efl_event_callback_array_del(client, _efl_net_server_udp_client_cbs(),
server);
- efl_net_server_clients_count_set(server,
efl_net_server_clients_count_get(server) - 1);
-
+ efl_event_callback_del(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_udp_client_event_closed, server);
eina_hash_del(pd->clients, efl_net_socket_address_remote_get(client),
client);
- if (efl_parent_get(client) == server)
- efl_parent_set(client, NULL);
}
-EFL_CALLBACKS_ARRAY_DEFINE(_efl_net_server_udp_client_cbs,
- { EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_udp_client_event_closed });
-
EOLIAN static void
_efl_net_server_udp_efl_net_server_fd_process_incoming_data(Eo *o,
Efl_Net_Server_Udp_Data *pd)
{
@@ -356,7 +347,6 @@
_efl_net_server_udp_efl_net_server_fd_process_incoming_data(Eo *o, Efl_Net_Serve
}
client = efl_add(EFL_NET_SERVER_UDP_CLIENT_CLASS, o,
- efl_event_callback_array_add(efl_added,
_efl_net_server_udp_client_cbs(), o),
efl_io_closer_close_on_destructor_set(efl_added,
EINA_TRUE),
efl_net_socket_address_local_set(efl_added,
efl_net_server_address_get(o)),
@@ -378,18 +368,10 @@
_efl_net_server_udp_efl_net_server_fd_process_incoming_data(Eo *o, Efl_Net_Serve
efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_REJECTED, str);
return;
}
+ efl_event_callback_add(client, EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_udp_client_event_closed, o);
- efl_net_server_clients_count_set(o, efl_net_server_clients_count_get(o) +
1);
- efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client);
-
- if (efl_ref_get(client) == 1) /* users must take a reference themselves */
- {
- DBG("client %s was not handled, closing it...",
- efl_net_socket_address_remote_get(client));
- free(buf);
- efl_del(client);
- return;
- }
+ if (!efl_net_server_client_announce(o, client))
+ return;
_efl_net_server_udp_client_feed(client, slice);
}
diff --git a/src/lib/ecore_con/efl_net_server_unix.c
b/src/lib/ecore_con/efl_net_server_unix.c
index 288c530..53deb4d 100644
--- a/src/lib/ecore_con/efl_net_server_unix.c
+++ b/src/lib/ecore_con/efl_net_server_unix.c
@@ -223,29 +223,10 @@ _efl_net_server_unix_efl_net_server_serve(Eo *o,
Efl_Net_Server_Unix_Data *pd, c
return _efl_net_server_unix_bind(o, pd);
}
-static Efl_Callback_Array_Item *_efl_net_server_unix_client_cbs(void);
-
-static void
-_efl_net_server_unix_client_event_closed(void *data, const Efl_Event *event)
-{
- Eo *server = data;
- Eo *client = event->object;
-
- efl_event_callback_array_del(client, _efl_net_server_unix_client_cbs(),
server);
- if (efl_parent_get(client) == server)
- efl_parent_set(client, NULL);
-
- efl_net_server_clients_count_set(server,
efl_net_server_clients_count_get(server) - 1);
-}
-
-EFL_CALLBACKS_ARRAY_DEFINE(_efl_net_server_unix_client_cbs,
- { EFL_IO_CLOSER_EVENT_CLOSED,
_efl_net_server_unix_client_event_closed });
-
static void
_efl_net_server_unix_efl_net_server_fd_client_add(Eo *o,
Efl_Net_Server_Unix_Data *pd EINA_UNUSED, int client_fd)
{
Eo *client = efl_add(EFL_NET_SOCKET_UNIX_CLASS, o,
- efl_event_callback_array_add(efl_added,
_efl_net_server_unix_client_cbs(), o),
efl_io_closer_close_on_exec_set(efl_added,
efl_net_server_fd_close_on_exec_get(o)),
efl_io_closer_close_on_destructor_set(efl_added,
EINA_TRUE),
efl_loop_fd_set(efl_added, client_fd));
@@ -256,15 +237,7 @@ _efl_net_server_unix_efl_net_server_fd_client_add(Eo *o,
Efl_Net_Server_Unix_Dat
return;
}
- efl_net_server_clients_count_set(o, efl_net_server_clients_count_get(o) +
1);
- efl_event_callback_call(o, EFL_NET_SERVER_EVENT_CLIENT_ADD, client);
-
- if (efl_ref_get(client) == 1) /* users must take a reference themselves */
- {
- DBG("client %s was not handled, closing it...",
- efl_net_socket_address_remote_get(client));
- efl_del(client);
- }
+ efl_net_server_client_announce(o, client);
}
static void
--
