On Wed, Aug 12, 2009 at 06:12:23PM -0400, Rishi Srivatsavai wrote: > Based on code review comments I have made the suggested changes > and created new webrevs for review. Webrevs are:
I looked through the updated webrev (mostly trill.c), and I have a couple of comments. It looks like it is possible for multiple threads to end up in trill_stop_recv(); consider the scenario where you have the "link destroyed" callback being triggered, but it ends up sleeping in trill_stop_recv() due to pending writes. When the last write completes, but before the callback thread wakes up, a close() comes in and calls trill_stop_recv(), which completes. Finally the callback thread wakes up, it calls bridge_trill_lnunref with a NULL pointer. So it looks like you should recheck the "link is NULL" condition after waking up. Another issue in trill_stop_recv() is that you are holding ts_socklock while calling bridge_trill_lnunref(), which might lead to a deadlock. The deadlock could happen if there are pending receive callbacks when close() is being called, which would cause the close() thread to go to sleep in bridge_trill_lnunref() (while holding the lock). In the unlikely scenario of the socket being flow controlled, the receive callback would try to grab ts_socklock. Cheers, Anders _______________________________________________ networking-discuss mailing list [email protected]
