use proper function for state destruction #256
Conversation
library/net.c
Outdated
| @@ -1217,7 +1217,7 @@ struct dnet_net_state *dnet_state_create(struct dnet_node *n, | |||
| err_out_dup_destroy: | |||
| dnet_sock_close(n, st->write_s); | |||
| err_out_free: | |||
| free(st); | |||
| dnet_state_destroy(st); | |||
There was a problem hiding this comment.
dnet_state_destroy calls pthread_mutex_lock(&st->trans_lock) as example. It'll be work correctly after destroy of mutex?
There was a problem hiding this comment.
Yes, you're right. I will add WIP label and rework it
| dnet_state_put(st); | ||
| pthread_mutex_destroy(&st->send_lock); | ||
| pthread_mutex_destroy(&st->trans_lock); | ||
| dnet_state_put(st); |
There was a problem hiding this comment.
It's undetermined what refcnt will left after dnet_state_move_to_dht call. In case of error, dnet_state_move_to_dht can put (-EEXIST) or not to put (other errorcode) the net state. So you can't know how many puts you must do at this line.
I think the good practic is to do like shared_ptr: we lock the resource in the component where we use this resource (in dnet_state_create), and don't do refcounting for another components. These another components must do clear refcounting by themself. For example: dnet_state_create must have scoped single refcnt-lock, and dnet_state_move_to_dht should inc (if success) or don't inc (if fail) refcnt. The same way in dnet_state_create: if success, we return net state and don't decrement refcnt; if fail, decrement refcnt and return NULL. The caller of dnet_state_create must keep the result object, or throw it away via put.
dnet_state_destroy()releases memory allocated forst->addrsand fillsst's memory with0xff.