On 19 aug 2013, at 18:11, Gerard Ziemski <gerard.ziem...@oracle.com> wrote:
> hi Staffan, > > A search on the topic of Mac thread id reveals that pretty much everyone > (including Apple itself) uses pthread_mach_thread_np() for thread id, so it > looks like you're correct to use it. > > I do have some more quick feedback/questions: > > 1. Could the "just checking" string in guarantee() be set to something more > informative, like "thread id doesn't exist"? I'll fix that. > 2. Why are we using the the "::" C++ name space before mach/pthread C APIs > calls? I understand that you might be just following the existing pattern in > the file, I'm just wondering if you, or anyone knows why. > > 3. Also not directly related to your fix, but do you know why we need both > "set_thread_id" and "set_unique_thread_id" The "unique" (not a good name) id is used by SA when matching threads. See JDK-8006423 for a longer discussion. The reason we don't want to use the "unique" id as thread_id is that it is a 64 bit number. Thanks, /Staffan > > > cheers > > > On 8/19/2013 4:08 AM, Staffan Larsen wrote: >> We are using the mach thread port name as the os thread identifier on OS X. >> We get this value by calling mach_thread_self(), but we (I) didn't realize >> that this "port" resource is reference counted and mach_thread_self() >> increases the reference count, but we never call mach_port_deallocate() to >> decrease the reference count. This leads to a resource leak and eventually >> mach_thread_self() will return 0 to indicate that we are out of ports. >> Running out of ports causes the pthreads implementation (which is built on >> top of mach) to spin forever in some calls (most notably in this case >> pthread_kill()). >> >> One way to fix this is to make sure we call mach_port_deallocate() for each >> call to mach_thread_self(). However, this adds extra complexity to the code >> and also makes it slower. Another solution is to ask pthreads for the mach >> port name that it already has. Pthreads provides the function >> pthread_mach_thread_np() for this purpose. In this case there is no need to >> deallocate the port since pthread_mach_thread_np() will not increase the >> reference count (and pthreads will call deallocate on the port once the >> thread terminates). >> >> This fix has been verified by seeing that the number of ports the process >> has allocated does not increase in Activity Monitor (or top). It has also >> passed JPRT testing on all platforms. >> >> webrev: http://cr.openjdk.java.net/~sla/8022808/webrev.00/ >> (there is not publicly visible bug for this) >> >> Thanks, >> /Staffan >
