lpereira / lwan

Experimental, scalable, high performance HTTP server
https://lwan.ws
GNU General Public License v2.0
5.94k stars 548 forks source link

Bug: lwan_trie_node_destroy shoud not free user data with hash_free #126

Closed matt-42 closed 8 years ago

matt-42 commented 8 years ago

You should let the user free the user provided data in lwan_url_map_t and not try to free it with hash_free here: https://github.com/lpereira/lwan/blob/af26c13b65d8e11c439d98f911dc7e22dc87d320/common/lwan.c#L135.

It seems like the freegoip.c is affected by this problem: https://github.com/lpereira/lwan/blob/07a19fc99826e0aafecbb0495e9cfe5f78179586/freegeoip/freegeoip.c#L429

The free is located here: https://github.com/lpereira/lwan/blob/dcd0431a1d2b8cb0128d964a68a3e7934d8641de/common/lwan-trie.c#L166

A workaround that worked for me: Manually unsetting the data pointer before shuting down the server:

`

inline void lwan_unset_urlmap_data_rec(lwan_trie_t trie, lwan_trie_node_t node) { if (!node) return;

int32_t nodes_unset = node->ref_count;

for (lwan_trie_leaf_t *leaf = node->leaf; leaf;) {
  lwan_trie_leaf_t *tmp = leaf->next;

  lwan_url_map_t* urlmap = (lwan_url_map_t*) leaf->data;
  urlmap->data = NULL;

  leaf = tmp;
}

for (int32_t i = 0; nodes_unset > 0 && i < 8; i++) {
  if (node->next[i]) {
    lwan_unset_urlmap_data_rec(trie, node->next[i]);
    --nodes_unset;
  }
}

}

inline void lwan_unset_urlmap_data(lwan_trie_t *trie) { if (!trie || !trie->root) return;

lwan_unset_urlmap_data_rec(trie, trie->root);    

}

`

lpereira commented 8 years ago

Good catch. I believe I've fixed this another way, by using a flag in the url_map to signal if data is a hash table or not. This is one of the things that will eventually need to be refactored, but for now this'll do. I'll push the patch soon.