<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2217/">https://reviewboard.asterisk.org/r/2217/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 28th, 2012, 11:53 a.m., <b>David Lee</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2217/diff/2/?file=32250#file32250line175" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/uuid.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ast_uuid_init(void)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">175</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">uuid_generate_random</span><span class="p">(</span><span class="n">uu</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It would be useful to test for the existing of /dev/urandom and log an error if it doesn't exist.
/dev/random is a blocking entropy source, and could dramatically affect performance if that's the only generator available.
If neither are available, libuuid uses rand(), which isn't thread safe.</pre>
</blockquote>
<p>On November 28th, 2012, 3:24 p.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This comment got me looking into libuuid a bit more.
If libuuid uses /dev/random/, it actually sets the reads to be nonblocking. If it fails to read data 16 times in a row, then it will fall back to simply using rand() for the current UUID instead. So performance is not degraded as much as it might be if it blocked while trying to read from /dev/random/
I took a look at the libuuid source a bit further, and even when getting data from /dev/random/ or /dev/urandom/, it actually uses the rand() (and possibly some use of jrand48()) function to do some adjustment on the returned bytes. If rand() is not thread-safe, then it would appear that *all* calls to uuid_generate_random() are not thread-safe due to their unprotected calls to rand().
It appears that we may need to surround all calls to uuid_generate_random() with a mutex. This may hurt performance some, but given all the locking Asterisk already is doing, I don't think this will be a bottleneck. What do you think?</pre>
</blockquote>
<p>On November 28th, 2012, 4:03 p.m., <b>David Lee</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In practice, if we're using /dev/urandom, this isn't likely to be a problem. libuuid is essentially generating a high quality, 128-bit random number. It then proceeds to XOR that number with a low quality random number generated by rand(). Since XOR preserves entropy, you still have a high quality random number.
This assumes that the thread-unsafe misbehaviors are minor (rand() isn't as random as it should be) and not major (rand() sends a message to SkyNet to start the revolution, posts your bank account information to Twitter, and then formats your hard drive, all the while displaying a laughing, highly pixelated Jolly Roger on the screen).
I dislike the idea of this becoming a contention point in the system, since it will likely be used by several different components. Although, the lock would help in the off chance libuuid falls back to using rand(). Maybe only lock if /dev/urandom is unavailable. Or maybe that's an optimization we can do if we decide that contention on generating UUID's is causing us trouble.
Sorry, I have no answers for you. Just vague ramblings.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The manpage for rand says this:
"The function rand() is not reentrant or thread-safe, since it uses hidden state that is modified on each call. This might just be the seed value to be used by the next call, or it might be something more elaborate. In order to get reproducible behavior in a threaded application, this state must be made explicit; this can be done using the reentrant function rand_r()."
This is a touch vague, but the only thing it mentions as far as the consequences of its non-thread-safety is that you cannot reliably reproduce results in multi-threaded applications unless the reentrant version is used. It doesn't mention anything about potential undefined behavior, so I have confidence the Skynet scenario you mentioned is unlikely.
Interestingly, have a look at Asterisk's ast_random() function in main/utils.c. It uses /dev/urandom/ if possible, but otherwise it will call random(). If on Linux, this is not protected with a lock, but on non-Linux systems, there is a lock around the random() call. What does this tell me? Not much, really, but still it's interesting.</pre>
<br />
<p>- Mark</p>
<br />
<p>On November 28th, 2012, 10:16 a.m., Mark Michelson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Matt Jordan and David Lee.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated Nov. 28, 2012, 10:16 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the first of many parts of the overall goal of improving Asterisk APIs. This adds a UUID API to Asterisk, eventually to be used as a method of uniquely identifying channels and potentially other objects.
The implementation here uses libuuid, which is packaged by all major Linux distributions. The API is basically a wrapper around libuuid's API, but it hides all implementation details in case we want to switch out libraries at some point.
Licensing for libuuid is a bit confusing. On one hand, this online man page (http://linux.die.net/man/3/libuuid) claims that the library is distributed under the LGPL version 2. However, when I downloaded the source for the library on my machine, the code had the 3-clause BSD license printed in the source. In either case, I believe use of external library is license-compatible with Asterisk.
Regarding specific implementation decisions, I think comments in the code should explain why I chose to do things certain ways. Please let me know if anything should be done differently or if you have ideas for tests beyond what I have already included.
I have no idea what is up with the changes listed in autoconfig.h.in. Those changes showed up when I ran the bootstrap.sh script after changing configure.ac. </pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have included a set of unit tests that exercise all the APIs. They pass when run locally.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20725">ASTERISK-20725</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/configure <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/configure.ac <span style="color: grey">(376724)</span></li>
<li>/trunk/include/asterisk/autoconfig.h.in <span style="color: grey">(376724)</span></li>
<li>/trunk/include/asterisk/uuid.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/main/Makefile <span style="color: grey">(376724)</span></li>
<li>/trunk/main/asterisk.c <span style="color: grey">(376724)</span></li>
<li>/trunk/main/uuid.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/tests/test_uuid.c <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2217/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>