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.
This commit is contained in:
Gustavo Sverzut Barbieri 2016-11-25 05:32:16 -02:00
parent c534d79124
commit 410d65900c
8 changed files with 170 additions and 79 deletions

View File

@ -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.

View File

@ -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"

View File

@ -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;
}
}

View File

@ -19,6 +19,67 @@ typedef struct _Efl_Net_Server_Ssl_Data
Eo *ssl_ctx;
} 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)
{
@ -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

View File

@ -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;

View File

@ -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

View File

@ -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);
}

View File

@ -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