[llvm] r241012 - Reapply "Use gethostuuid() on Mac to identify hosts for LockFileManager"

[Eric Christopher]

On Mon, Jun 29, 2015 at 3:50 PM Ben Langmuir <blangmuir at apple.com> wrote:

> On Jun 29, 2015, at 3:28 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> On Mon, Jun 29, 2015 at 3:26 PM Ben Langmuir <blangmuir at apple.com> wrote:
>
>> On Jun 29, 2015, at 3:20 PM, Eric Christopher <echristo at gmail.com> wrote:
>>
>> Uh...
>>
>> Why not just check for the header and use that?
>>
>>
>> gethostuuid() is defined in unistd.h on recent versions of Mac.  The
>> presence of uuid.h is necessary but not sufficient.
>>
>>
> IIRC I put in some other checks for uuid.h at one point, you might want to
> audit it.
>
>
> I can’t find any other use of uuid.h in llvm or clang.  Do you recall
> where this was?
>
>
IIRC it was support, but it's been a few years.


>
> That said, you can still check for the function in the available headers...
>
>
>> Or check for the functions you need etc at configure/cmake time?
>>
>>
>> Sadly, on some platforms and/or versions where this function exists, it
>> returns a bogus value.
>>
>>
> Released versions?
>
>
> Yes.
>
>
Whee. That said...


>
> If so, this is just bogus all over the place. :\ That said you could
> restrict the use by known good versions or write checks to make sure that
> it's valid to use rather than #ifdef'ing them.
>
>
> I’m not sure I understand - are you suggesting moving the
> platform-specific #ifdef I’m using into the configuration stage, or are you
> suggesting checking the validity of the return values directly?  If the
> latter, then unfortunately the “bad” values aren’t readily distinguishable,
> they’re just not “unique” enough.
>
>
Since you've got a version of it you can still make it a configure time
check rather than a compile one to keep the amount of version specific
ifdefs out of the file. If you need help with the autoconf side I can help,
the cmake side... I'll punt to Chris B or someone. :)

-eric


> Ben
>
>
> -eric
>
>
>>
>> -eric
>>
>> On Mon, Jun 29, 2015 at 3:19 PM Ben Langmuir <blangmuir at apple.com> wrote:
>>
>>> Author: benlangmuir
>>> Date: Mon Jun 29 17:16:39 2015
>>> New Revision: 241012
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=241012&view=rev
>>> Log:
>>> Reapply "Use gethostuuid() on Mac to identify hosts for LockFileManager"
>>>
>>> Reapplies r241005 after fixing the build on non-Mac platforms. Original
>>> commit message below.
>>>
>>> The hostname can be very unstable when there are many machines on the
>>> network competing for the same name. Using the hardware UUID makes it
>>> less likely to have collisions or to consider files written by the
>>> current host to be owned by a different one at a later time.
>>>
>>> rdar://problem/21512307
>>>
>>> Modified:
>>>     llvm/trunk/lib/Support/LockFileManager.cpp
>>>
>>> Modified: llvm/trunk/lib/Support/LockFileManager.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=241012&r1=241011&r2=241012&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Support/LockFileManager.cpp (original)
>>> +++ llvm/trunk/lib/Support/LockFileManager.cpp Mon Jun 29 17:16:39 2015
>>> @@ -21,6 +21,16 @@
>>>  #if LLVM_ON_UNIX
>>>  #include <unistd.h>
>>>  #endif
>>> +
>>> +#if defined(__APPLE__) && defined(__MAC_OS_X_VERSION_MIN_REQUIRED) &&
>>> (__MAC_OS_X_VERSION_MIN_REQUIRED > 1050)
>>> +#define USE_OSX_GETHOSTUUID 1
>>> +#else
>>> +#define USE_OSX_GETHOSTUUID 0
>>> +#endif
>>> +
>>> +#if USE_OSX_GETHOSTUUID
>>> +#include <uuid/uuid.h>
>>> +#endif
>>>  using namespace llvm;
>>>
>>>  /// \brief Attempt to read the lock file with the given name, if it
>>> exists.
>>> @@ -56,14 +66,45 @@ LockFileManager::readLockFile(StringRef
>>>    return None;
>>>  }
>>>
>>> -bool LockFileManager::processStillExecuting(StringRef Hostname, int
>>> PID) {
>>> +static std::error_code getHostID(SmallVectorImpl<char> &HostID) {
>>> +  HostID.clear();
>>> +
>>> +#if USE_OSX_GETHOSTUUID
>>> +  // On OS X, use the more stable hardware UUID instead of hostname.
>>> +  struct timespec wait = {1, 0}; // 1 second.
>>> +  uuid_t uuid;
>>> +  if (gethostuuid(uuid, &wait) != 0)
>>> +    return std::error_code(errno, std::system_category());
>>> +
>>> +  uuid_string_t UUIDStr;
>>> +  uuid_unparse(uuid, UUIDStr);
>>> +  StringRef UUIDRef(UUIDStr);
>>> +  HostID.append(UUIDRef.begin(), UUIDRef.end());
>>> +
>>> +#elif LLVM_ON_UNIX
>>> +  char HostName[256];
>>> +  HostName[255] = 0;
>>> +  HostName[0] = 0;
>>> +  gethostname(HostName, 255);
>>> +  StringRef HostNameRef(HostName);
>>> +  HostID.append(HostNameRef.begin(), HostNameRef.end());
>>> +
>>> +#else
>>> +  StringRef Dummy("localhost");
>>> +  HostID.append(Dummy.begin(), Dummy.end());
>>> +#endif
>>> +
>>> +  return std::error_code();
>>> +}
>>> +
>>> +bool LockFileManager::processStillExecuting(StringRef HostID, int PID) {
>>>  #if LLVM_ON_UNIX && !defined(__ANDROID__)
>>> -  char MyHostname[256];
>>> -  MyHostname[255] = 0;
>>> -  MyHostname[0] = 0;
>>> -  gethostname(MyHostname, 255);
>>> +  SmallString<256> StoredHostID;
>>> +  if (getHostID(StoredHostID))
>>> +    return true; // Conservatively assume it's executing on error.
>>> +
>>>    // Check whether the process is dead. If so, we're done.
>>> -  if (MyHostname == Hostname && getsid(PID) == -1 && errno == ESRCH)
>>> +  if (StoredHostID == HostID && getsid(PID) == -1 && errno == ESRCH)
>>>      return false;
>>>  #endif
>>>
>>> @@ -126,17 +167,18 @@ LockFileManager::LockFileManager(StringR
>>>
>>>    // Write our process ID to our unique lock file.
>>>    {
>>> -    raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
>>> +    SmallString<256> HostID;
>>> +    if (auto EC = getHostID(HostID)) {
>>> +      Error = EC;
>>> +      return;
>>> +    }
>>>
>>> +    raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
>>> +    Out << HostID << ' ';
>>>  #if LLVM_ON_UNIX
>>> -    // FIXME: move getpid() call into LLVM
>>> -    char hostname[256];
>>> -    hostname[255] = 0;
>>> -    hostname[0] = 0;
>>> -    gethostname(hostname, 255);
>>> -    Out << hostname << ' ' << getpid();
>>> +    Out << getpid();
>>>  #else
>>> -    Out << "localhost 1";
>>> +    Out << "1";
>>>  #endif
>>>      Out.close();
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150630/c5326a20/attachment.html>