[llvm] r241012 - Reapply "Use gethostuuid() on Mac to identify hosts for LockFileManager"
Eric Christopher
echristo at gmail.com
Thu Jul 2 18:08:00 PDT 2015
On Thu, Jul 2, 2015 at 9:00 AM Ben Langmuir <blangmuir at apple.com> wrote:
> On Jun 30, 2015, at 11:50 AM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> 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. :)
>
>
> I took a look this morning, but it wasn’t obvious to me how I should set
> up such a check in either the autoconf or cmake builds. I see some checks
> for *tool* versions, but not OS versions. I guess one way would be to just
> use my existing platform-specific #if, but in a configure-time test program
> that #errors for unsupported cases… Is there a better way?
>
>
Nope. Barring any way of knowing whether or not the API works other than
version numbers that's the best way.
-eric
> Thanks,
>
> Ben
>
>
> -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/20150703/ec10385c/attachment.html>
More information about the llvm-commits
mailing list