Re: Release date for Privoxy 3.0.24
Ian Silvester <[email protected]> Thu, 21 Jan 2016 14:57:53 -0500
| Newsgroups | gmane.comp.web.privoxy.devel |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format. --===============9153943421387923455== Content-Type: multipart/alternative; boundary="------------080307060102030605070606" This is a multi-part message in MIME format. --------------080307060102030605070606 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 2016-01-21 13:48, Fabian Keil wrote: > Ian Silvester <[email protected]> wrote: > >> On 2016-01-21 11:46, Fabian Keil wrote: >>> Ian Silvester <[email protected]> wrote: >>> >>>> I have tested removal of the get_thread_id MACH IFDEF. It is indeed >>>> strictly unnecessary since even without it the threads are still >>>> uniquely identified. I propose however that it is kept and modified as >>>> per the compiler warning, since the raw IDs use four extra bytes which >>>> could better be used by the remainder of the log line, plus the comment >>>> in the code is strictly true. >>>> >>>> Let me know if anyone disagrees. >>> I don't disagree with your labs() commit which seems to be >>> the best solution before the release. >>> >>> My main objection to the MACH section in general is that >>> "this_thread % 1000" actually increases the chances of id >>> collisions and that it's only done on MACH while the issue >>> it's supposed to address exists on other platforms as well. >>> >>> Eventually it may make sense to figure out a hashing scheme >>> that works cross-platform and is less likely to cause collisions. >> Unless you know more than what is said in the comment, I read it to say >> that the issue has only been shown to affect OS X, but probably affects >> other OSes that use the MACH kernel. >> >> Are you certain that collisions could occur as-is? It seems a remote >> possibility to me. > Unless I miss something, after the "% 1000" there can't be more than > 1000 unique values left and as a result there must be collisions if > there are more than 1000 different threads ids. > > In practice the number of required threads to get collisions is probably > a lot lower, as the thread ids aren't assigned randomly or by incrementing > the previous thread id by one. > > I suspect that the only guarantee is that two threads that are running > at the same time have different thread ids. > >> If you are sure, perhaps rather than add the better hashing to our >> already insanely long TODO list I simply remove the labs function call? > The change from abs() to labs() should have no affect on the collision > "risk". > > BTW, it seems like we may be able to solve the issue by using > pthread_getthreadid_np() instead of pthread_self() in the future. > >> This would leave us with guaranteed-unique 9 byte long thread ids >> resulting from having their rightmost three 0s trimmed. > I don't think so. > >> These may still >> not be unique in their first 4 bytes (though they're significantly more >> different than the raw ids); I'll admit I don't understand the comment's >> talk of a "debuggable value in the first 4 bytes" - do you think it >> simply means a unique value in those 4 bytes? And what is so important >> about the first 4 bytes of the value from a debugging perspective? > While I'm having trouble parsing the comment as well, I interpret it similarly. > I assume David meant to write "discernible" or "distinguishable" or something > like this and was using a 32 bit system which is relevant for the "4 bytes" part. > > Fabian I wrote that comment. 'debuggable' literally meant just that. If you wanted to attach gdb, for example, you'd want to know which process ID to attach to... but of course we're talking about threads, not processes. So debuggability is actually moot here. The idea was simply to make the thread ID look somewhat presentable as a number, and still have a reasonable chance of not colliding with another similarly truncated thread ID (in Privoxy). The Mach kernel has a very chatty thread ID, so I was just trying to reduce clutter in the log. - David Thanks for the input David! So the upshot is that we don't need to worry about debuggability (I'll modify the comment appropriately) and only need to ensure no numbering collision whilst indeed making the log more readable. Fabian, given the style of the raw ids (e.g. 7fff78fb5000) the '% 1000' has the effect of simply stripping the least significant three digits, which are always 000. This alone then does not affect the uniqueness of the IDs. e.g. 7fff78fb5000 becomes 7fff78fb5, right? Since they remain unique at that stage, taking their absolute value as long ints will still give a unique value - I think we are safe here with the abs->labs change. All that needs to change is the comment ;o) Unfortunately pthread_getthreadid_np() is not available on OS X. Here we have pthread_threadid_np() instead which I cannot recommend, both because the syntax is significantly different and because it is only available since 10.6, which'd cut off OS X versions we currently support. Ian -- My PGP public key <http://diem.serveftp.net:8080/IanSilvesterPGPPublicKey.asc>. --------------080307060102030605070606 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit <html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <br> <br> <div class="moz-cite-prefix">On 2016-01-21 13:48, Fabian Keil wrote:<br> </div> <blockquote cite="mid:[email protected]" type="cite"> <pre wrap="">Ian Silvester <a class="moz-txt-link-rfc2396E" href="mailto:[email protected]"><[email protected]></a> wrote: </pre> <blockquote type="cite"> <pre wrap="">On 2016-01-21 11:46, Fabian Keil wrote: </pre> <blockquote type="cite"> <pre wrap="">Ian Silvester <a class="moz-txt-link-rfc2396E" href="mailto:[email protected]"><[email protected]></a> wrote: </pre> <blockquote type="cite"> <pre wrap="">I have tested removal of the get_thread_id MACH IFDEF. It is indeed strictly unnecessary since even without it the threads are still uniquely identified. I propose however that it is kept and modified as per the compiler warning, since the raw IDs use four extra bytes which could better be used by the remainder of the log line, plus the comment in the code is strictly true. Let me know if anyone disagrees. </pre> </blockquote> <pre wrap="">I don't disagree with your labs() commit which seems to be the best solution before the release. My main objection to the MACH section in general is that "this_thread % 1000" actually increases the chances of id collisions and that it's only done on MACH while the issue it's supposed to address exists on other platforms as well. Eventually it may make sense to figure out a hashing scheme that works cross-platform and is less likely to cause collisions. </pre> </blockquote> <pre wrap="">Unless you know more than what is said in the comment, I read it to say that the issue has only been shown to affect OS X, but probably affects other OSes that use the MACH kernel. Are you certain that collisions could occur as-is? It seems a remote possibility to me. </pre> </blockquote> <pre wrap=""> Unless I miss something, after the "% 1000" there can't be more than 1000 unique values left and as a result there must be collisions if there are more than 1000 different threads ids. In practice the number of required threads to get collisions is probably a lot lower, as the thread ids aren't assigned randomly or by incrementing the previous thread id by one. I suspect that the only guarantee is that two threads that are running at the same time have different thread ids. </pre> <blockquote type="cite"> <pre wrap="">If you are sure, perhaps rather than add the better hashing to our already insanely long TODO list I simply remove the labs function call? </pre> </blockquote> <pre wrap=""> The change from abs() to labs() should have no affect on the collision "risk". BTW, it seems like we may be able to solve the issue by using pthread_getthreadid_np() instead of pthread_self() in the future. </pre> <blockquote type="cite"> <pre wrap="">This would leave us with guaranteed-unique 9 byte long thread ids resulting from having their rightmost three 0s trimmed. </pre> </blockquote> <pre wrap=""> I don't think so. </pre> <blockquote type="cite"> <pre wrap=""> These may still not be unique in their first 4 bytes (though they're significantly more different than the raw ids); I'll admit I don't understand the comment's talk of a "debuggable value in the first 4 bytes" - do you think it simply means a unique value in those 4 bytes? And what is so important about the first 4 bytes of the value from a debugging perspective? </pre> </blockquote> <pre wrap=""> While I'm having trouble parsing the comment as well, I interpret it similarly. I assume David meant to write "discernible" or "distinguishable" or something like this and was using a 32 bit system which is relevant for the "4 bytes" part. Fabian </pre> </blockquote> <blockquote><br> <pre wrap="">I wrote that comment. <span class="moz-smiley-s1" title=":-)"></span> 'debuggable' literally meant just that. If you wanted to attach gdb, for example, you'd want to know which process ID to attach to... but of course we're talking about threads, not processes. So debuggability is actually moot here. The idea was simply to make the thread ID look somewhat presentable as a number, and still have a reasonable chance of not colliding with another similarly truncated thread ID (in Privoxy). The Mach kernel has a very chatty thread ID, so I was just trying to reduce clutter in the log. - David</pre> </blockquote> <br> <br> Thanks for the input David!<br> <br> So the upshot is that we don't need to worry about debuggability (I'll modify the comment appropriately) and only need to ensure no numbering collision whilst indeed making the log more readable.<br> <br> Fabian, given the style of the raw ids (e.g. 7fff78fb5000) the '% 1000' has the effect of simply stripping the least significant three digits, which are always 000. This alone then does not affect the uniqueness of the IDs. e.g. 7fff78fb5000 becomes 7fff78fb5, right?<br> <br> Since they remain unique at that stage, taking their absolute value as long ints will still give a unique value - I think we are safe here with the abs->labs change. All that needs to change is the comment ;o)<br> <br> Unfortunately pthread_getthreadid_np() is not available on OS X. Here we have pthread_threadid_np() instead which I cannot recommend, both because the syntax is significantly different and because it is only available since 10.6, which'd cut off OS X versions we currently support. <meta charset="utf-8"> <br> <br> Ian<br> <br> <br> <div class="moz-signature">-- <br> <a href="http://diem.serveftp.net:8080/IanSilvesterPGPPublicKey.asc">My PGP public key</a>.</div> </body> </html> --------------080307060102030605070606-- --===============9153943421387923455== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 --===============9153943421387923455== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Ijbswa-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ijbswa-developers --===============9153943421387923455==--