NVIDIA / nccl

Optimized primitives for collective multi-GPU communication
Other
3.28k stars 831 forks source link

Fix double free in ncclTopoRemovePathType #1354

Closed junxu closed 4 months ago

kiskra-nvidia commented 4 months ago

Thank you for your submission!

Your patch, while harmless, doesn't seem necessary to me -- is it based on an actual problem you encountered?

https://github.com/NVIDIA/nccl/blob/b9d748672478a1d2dd2e67b0c5e518b5e075dc98/src/graph/paths.cc#L202-L210

The reason I'm asking is that right above, in line 203, we set node->paths[nodeType] to NULL, so the subsequent free in line 209 should operate on a NULL pointer -- while many programmers like to check for that, in principle free(NULL) has a well defined semantics (it's harmless). Or am I missing something?

Anyway, this part of the code will have changed for the next NCCL release so your PR is not applicable (sorry!).