I have not read this patch carefully so I have no idea that the patch solved the problem or not. I just have two question:
1) do we find the cause of the crash? (how to trigger the crash, in what situation) 2) is it worth using smart pointer to prevent the crash? (maintain the design principle of the ATS) If we can not find a perfect solution now， why not just put it aside. BTW: In taobao`s tree (base on 3.0.1 and some patches of our own), we have not seen the crash for a long time. On Tue, 2012-03-13 at 12:36 -0500, Alan M. Carroll wrote: > I have a patch submitted for TS-857 which attempts to make a start on finer > grained locking. The essence of it is > > 1) Provide more powerful and standards compliant reference counted smart > pointers - lib/ts/IntrusivePtr.h > > 2) Provide reference counted lists that are compatible with both (1) and > existing intrusive list support. > > The current IntrusivePtr.h does more than this but that is an artifact of the > development process, as I will describe later. > > For TS-857 in particular, the change is to replace the existing NetHandler > open_list with a reference counted list which uses the same internal links, > and to change HttpServerSession to use reference counted smart pointers. > These changes mean that NetHandler can detect when a VC is shared and avoid > closing it underneath the other thread. The current Handle and callback > mechanism is discarded because it's a hack. > > Unfortunately, for reasons I don't understand, I can no longer replicate the > problem on my test setup, even going back to 3.1.1 where as before I could > replicate it in under a minute. So, while this patch seems stable I can not > demonstrate that it is better with respect to TS-857. It passes regression, > my load testing, and does not seem to leak connections (traffic_line reports > a constant number of open connections even after much load testing). > > Reference counting is implemented only for UnixNetVConnections as the > NetHandler open list contains only instances of that type. Unfortunately the > actual mechanism must be introduced in to the class tree at NetVConnection > because that is what HttpServerSession uses. The normal reference counting > cleanup is disabled at this point because it's either handled differently > (UnixNetVConnection) or not wanted (all other NetVConnection subclasses). > > The additional types in IntrusivePtr.h that are not used were used during > earlier work but it seemed later that changing only open_list was the best > approach. The other types were left in as they seem potentially useful. > > The question going forward is, should we commit this patch? Any comments are > appreciated. For now, my recommendation is to commit and resolve TS-857 > unless someone else can either replicate it or provide field report of its > occurrence. >
