<p dir="ltr">Great job!<br>
Thank you</p>
<div class="gmail_quote">On Jan 21, 2016 6:34 AM, "Markus Mayer" <<a href="mailto:code@mmayer.net">code@mmayer.net</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>There are several bug reports out there that squid is failing to start up on OS X with an error similar to this:</div><div><br></div>Ipc::Mem::Segment::create failed to ftruncate(/squid-cf__metadata.shm): (22) Invalid argument<br><div><br></div><div>It may be one of the other shared memory segments that is failing, but the problem is always the same: upon restarting squid, ftruncate() bails with EINVAL and squid subsequently aborts. If one is lucky and waits long enough, restarting squid may eventually succeed. Or one may need to reboot.</div><div><br></div><div>From what I was able to find, the root cause for this has never really been determined, and there hasn't been a fix in squid that addresses this. (It was still happening to me intermittently on OS X 10.11.2 and the most recent squid from GIT.)</div><div><br></div><div>So, I took some time to dig into this, down to first looking at the Darwin kernel sources (<a href="http://www.opensource.apple.com/source/xnu/xnu-3248.20.55/bsd/kern/posix_shm.c" target="_blank">http://www.opensource.apple.com/source/xnu/xnu-3248.20.55/bsd/kern/posix_shm.c</a>) and subsequently downloading XNU, instrumenting posix_shm.c and running my own custom built Darwin kernel (which was surprisingly straight forward). Since there are several places where ftruncate() and, specifically, pshm_truncate() can return EINVAL, instrumenting the kernel seemed to be the most promising way of figuring out which EINVAL squid is hitting and why.</div><div><br></div><div>Turns out it's this one:</div><div><br></div><div><div><span style="white-space:pre-wrap"> </span>if ((pinfo->pshm_flags & (PSHM_DEFINED|PSHM_ALLOCATING|PSHM_ALLOCATED)) </div><div><span style="white-space:pre-wrap"> </span>!= PSHM_DEFINED) {</div><div><span style="white-space:pre-wrap"> </span>PSHM_SUBSYS_UNLOCK();</div><div><span style="white-space:pre-wrap"> </span>return(EINVAL);</div><div><span style="white-space:pre-wrap"> </span>}</div></div><div><br></div><div>It happens because PSHM_ALLOCATED is already set. Luckily, the only place where PSHM_ALLOCATED gets set is at the end of pshm_truncate() itself. So, finding the culprit for setting it was easy.</div><div><br></div><div>What this means is that OS X will only support ftruncate() *ONCE* on a shm segment, and this is done fully on purpose. The first time pshm_truncate() is called, it'll do all the required mapping and so forth, then set PSHM_ALLOCATED for that segment. When one tries to call ftruncate() on the same shm region again, it'll return EINVAL, because PSHM_ALLOCATED is set.</div><div><br></div><div>There are cases where shm regions don't get removed when squid terminates. So, upon restarting squid, shm_open() will return the already existing shm segment (or segments) which will have been "allocated" already. squid will try to call ftruncate() anyway -- and fail.</div><div><br></div><div>Now, one could argue that OS X should really be able to handle ftruncate() on an existing shm segment (like other POSIX systems do), and it probably should. Convincing Apple of that is likely not going to be easy. Even if it were to succeed, it would only fix future releases, not the current OS X releases.</div><div><br></div><div>So, I see the following solutions squid could implement, which could be Darwin-only code, so as to not burden other OSes with this workaround.</div><div><br></div><div>- Try to ensure squid can't leave shm segments behind when it terminates. Not sure how realistic this is. It could always fail in a way that prevents it from cleaning up after itself. Or somebody could send it a SIGKILL, in which case it definitely couldn't clean up after itself.</div><div>- Simply accepting ftruncate() failing with EINVAL as non fatal condition on OS X and continue. This would be easy to do, but there are several different reasons why EINVAL may be returned. It wouldn't be a good idea to continue in all those cases. So this won't really work reliably.<br></div><div>- Run statSize() on the memory segment after successfully opening it. If the size returned is greater than 0, we know ftruncate() will fail on OS X, because the segment isn't a new one. So, squid could call ftruncate() only if size is 0 and maybe call memset() otherwise to zero it out.</div><div>- Just unconditionally call unlink() before shm_open(). If the segment didn't exist, unlink() will fail, but that won't really matter. This would likely take the least amount of code.<br></div><div><div>- Call shm_open with O_EXCL, so that it'll fail if the shm segment already exists. If the call fails with EEXIST, unlink the shm segment and retry. This one may not even need to be Darwin-specific. Would be unnecessary on other platforms, but wouldn't really hurt.<br></div></div><div><br></div><div>I did create a sample implementation of the last option. According to my tests, it does seem to work as intended.</div><div><br></div><div><div>--- a/src/ipc/mem/Segment.cc</div><div>+++ b/src/ipc/mem/Segment.cc</div><div>@@ -85,12 +85,31 @@ Ipc::Mem::Segment::Enabled()</div><div> void</div><div> Ipc::Mem::Segment::create(const off_t aSize)</div><div> {</div><div>+ bool do_retry = false;</div><div>+</div><div> assert(aSize > 0);</div><div> assert(theFD < 0);</div><div> </div><div>- // OS X does not allow using O_TRUNC here.</div><div>- theFD = shm_open(theName.termedBuf(), O_CREAT | O_RDWR,</div><div>- S_IRUSR | S_IWUSR);</div><div>+ do {</div><div>+ // OS X does not allow using O_TRUNC with shm_open.</div><div>+ // Also, OS X only permits ftruncate() on new shared memory areas.</div><div>+ // Therefore, we know that ftruncate() will fail if the shared memory</div><div>+ // area already exists. To prevent this, we delete and re-create the</div><div>+ // area if it exsisted previously (i.e. from an unclean shutdown).</div><div>+ theFD = shm_open(theName.termedBuf(), O_CREAT | O_EXCL | O_RDWR,</div><div>+ S_IRUSR | S_IWUSR);</div><div>+ if (theFD < 0 && errno == EEXIST) {</div><div>+ int old_errno = errno;</div><div>+ unlink();</div><div>+ // We want to report the shm_open failure, not the unlink failure.</div><div>+ errno = old_errno;</div><div>+ // Retry once, but only once.</div><div>+ do_retry = !do_retry;</div><div>+ } else {</div><div>+ do_retry = false;</div><div>+ }</div><div>+ } while (do_retry);</div><div>+</div><div> if (theFD < 0) {</div><div> debugs(54, 5, HERE << "shm_open " << theName << ": " << xstrerror());</div><div> fatalf("Ipc::Mem::Segment::create failed to shm_open(%s): %s\n",</div></div><div><br></div><div><br></div><div>Regards,</div><div>-Markus</div><div><br></div></div>
<br>_______________________________________________<br>
squid-dev mailing list<br>
<a href="mailto:squid-dev@lists.squid-cache.org">squid-dev@lists.squid-cache.org</a><br>
<a href="http://lists.squid-cache.org/listinfo/squid-dev" rel="noreferrer" target="_blank">http://lists.squid-cache.org/listinfo/squid-dev</a><br>
<br></blockquote></div>