Hi Patrik,
On 16.04.2012 11:39, Patrik Flykt wrote:
> On Thu, 2012-04-12 at 10:14 +0200, Daniel Wagner wrote:
>> From: Daniel Wagner <[email protected]>
>
> There seems to be a network structure refcount that never reaches zero
> in order to remove the network structure and thereby the corresponding
> service. Thus I unfortunately need to NACK this patch set for now until
> the refcount issue is fixed.
The service autoconnect part selects the service which is about to
disappear:
plugins/ethernet.c:
static void remove_network(struct connman_device *device,
struct ethernet_data *ethernet)
{
if (ethernet->network == NULL)
return;
connman_device_remove_network(device, ethernet->network);
connman_network_unref(ethernet->network);
ethernet->network = NULL;
}
When connman_device_remove_network() is called the service autoconnect
part reselects this network and the connman_network_unref() part in
etherenet.c reduces the refcount but since service as reselected the
network the refcount is not 0 here.
So the problem is that connman_device_remove_network() has to make sure
that this services which belongs to this network is not reselected. We
can argue on the fact how we want to expose this. The
__connman_network_is_valid() should be okay for the
service.c:is_ignored() part, I assume.
About the implementation: Assigning network->device = NULL as one of the
first things in the connman_device_remove_network() code path has the
benefit, that we don't have to touch other places where we do check for
network->device != NULL part. Which is used to tell the upper layers
that the network is ready. Using an additional flag to indicated if the
network object is ready to use should be avoided IMO.
The owner ship changes we did not too long ago have changed the creation
and destruction of the network/service objects to be symmetrical. This
is one of the leftovers. The reason we did not see this before is that
the autoconnect always could select the ethernet service. And now we are
also removing the ethernet service and therefore we see the refcount
life lock.
So what you are describing as underlaying problem is exactly what I fix
with this series.
cheers,
daniel
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
