Re: Program occasionally crashes on startup due to GNUstepDefaults.lck error

[Wolfgang Lux]

> Am 17.06.2017 um 08:04 schrieb Richard Frith-Macdonald 
> <richard.frith-macdonald@theengagehub.com>:
> 
> 
>> On 16 Jun 2017, at 10:48, Wolfgang Lux <wolfgang.lux@gmail.com> wrote:
>> 
>> 
>>> Am 14.06.2017 um 22:29 schrieb Lobron, David <dlobron@akamai.com>:
>>> 
>>> Hello GNUstep people,
>>> 
>>> I'm debugging an occasional crash that we've observed on machines that run 
>>> 20 instances of a GNUstep-based program.  The crash does not occur often, 
>>> and I can't reproduce it on demand, but when it does occur, we see a 
>>> message like this: 
>>> 
>>> mapmaker_7.exe: Uncaught exception NSGenericException, reason: Unable to 
>>> get attributes of lock file we made at 
>>> /root/GNUstep/Defaults/.lck/.GNUstepDefaults.lck
>>> 
>>> Since the lock file path is absolute, I suspect the problem here is 
>>> contention among the various processes for the same file if two or more of 
>>> them are starting up at exactly the same time.  However, I would expect a 
>>> lock file to be impervious to multiple readers.  Maybe there is a 
>>> non-atomic operation happening here.  I searched the code for 
>>> "GNUstepDefaults.lck", but I did find it there.  The 
>>> /root/GNUstep/Defaults/.lck is normally empty, so it seems like this is a 
>>> transient file.
>>> 
>>> Does anyone know if there's a recommended way to handle this?  Is it a 
>>> known issue?
>> 
>> Sort of. I've seen many issues revolving around GNUstepDefaults.lck in the 
>> past here.
>> However, looking at the code I'm afraid the reason is simply that the 
>> NSDistributedLock implementation is broken (and apparently is so for quite 
>> some time). :-(
>> The problem is that the code of NSDistributedLock was changed to use 
>> -[NSFileManager 
>> createDirectoryAtPath:withIntermediateDirectories:attributes:error:] with 
>> the second argument set to YES so that it would create intermediate 
>> directories. However, that means that this method returns YES regardless of 
>> whether the new directory already exists or not. I think changing the 
>> parameter to NO should fix your (and everyone else's) issue.
> 
> Thanks a lot for that Wolfgang, I've spent days trying to figure out why the 
> user defaults locks were occasionally failing, looking in completely the 
> wrong area (threading mostly)' and never considering it might be something as 
> basic as that.  It's great that you spotted the cause of the problem.
> I have changed the NSDistributedLock code (github) to use O/S level functions 
> to create the lock directory, to be sure that if a directory already exists 
> we see the creation attempt as failing rather than succeeding.

Thanks for committing the change for me, I was still in the process of 
switching my local checkout to git.
However, it looks like you got my comment slightly wrong. I didn't mean to say 
that using the 
createDirectoryAtPath:withIntermediateDirectories:attributes:error: method is 
wrong or that that method is broken in any way. It's just that calling it with 
the withIntermediateDirectories: argument set to YES is wrong because in that 
case the method return YES even if the directory already exists. Calling the 
method with NO for the second parameter would have been perfectly okay (unless 
the create method would indeed contain a bug).

Wolfgang