You are here: Foswiki>Tasks Web>Item14200 (30 Aug 2017, MichaelDaum)Edit Attach

Item14200: Potentially thread unsafe code in LdapContrib::refreshCache

Priority: Urgent
Current State: Confirmed
Released In: n/a
Target Release: n/a
Applies To: Extension
Component: LdapContrib
Reported By: FredTarbox
Waiting For:
Last Change By: MichaelDaum
I'm looking at:
sub refreshCache {
  my ($this, $mode) = @_;

  return unless $mode;

  $this->{_refreshMode} = $mode;

# create a temporary tie
  my $tempCacheFile = $this->{cacheFile} . '_tmp';
  if (-e $tempCacheFile) {
    writeWarning("already refreshing cache");
    return 0;

  my %tempData;
  my $tempCache = tie %tempData, 'DB_File', $tempCacheFile, O_CREAT | O_RDWR, 0664, $DB_HASH
    or die "Cannot open file $tempCacheFile: $!";

My understanding may be flawed. I believe tie is atomic, but DB_File is not DB_File::Lock, and O_CREAT will open an existing file unless you have O_EXCL. If two requests make it past the -e $tempCacheFile together, one will create the file and the other will open it but both will succeed and have access. Disaster may follow.

This also worries me:

  #writeDebug("replacing working copy");
  rename $tempCacheFile, $this->{cacheFile};

  # reconnect hash

Is rename atomic? Does it respect locks other processes might have on the file?

I would like these to be the cause of a formerly-once-a-year bug that corrupts groups on my system which has shown up twice in the past three weeks, it would mean I could stop looking.

-- FredTarbox - 17 Oct 2016

Setting to Confirmed. Jast has confirmed that there are indeed some thread safe issues in LdapContrib.

-- GeorgeClark - 02 Dec 2016

The real fix would be to not use BerkeleyDB for concurrent updates ... which isn't happening regularly anyway.

For now, the best way to avoid a race condition is to not update the cache online using an appropriate negative MaxCacheAge value and update the cache using a cron job at midnight. This is actually so highly recommended that I think of disabling online updates all together, means: you must use a cron job to update the ldap cache.

-- MichaelDaum - 30 Aug 2017

ItemTemplate edit

Summary Potentially thread unsafe code in LdapContrib::refreshCache
ReportedBy FredTarbox
SVN Range
AppliesTo Extension
Component LdapContrib
Priority Urgent
CurrentState Confirmed
TargetRelease n/a
ReleasedIn n/a
Topic revision: r4 - 30 Aug 2017, MichaelDaum
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy