2016-07-18 2 views
1

У меня есть Потокобезопасная класс:Возможная неправильная реализация двойной проверки блокировки

internal class ExmplFile 
{ 
    private readonly string filename; 
    private int resolution; 
    private volatile PaddedImage gaussian; 
    private object lockObject=new object(); 

    //blah, blah, blah 

    internal PaddedImage Gaussian() 
    { 
     if (gaussian != null) 
     { 
      return gaussian; 
     } 
     lock (lockObject) 
     { 
      if (gaussian == null) 
      { 
       Image(); 
       if (File.Exists(filename + "-gaus.raw")) 
       { 
        gaussian = LoadImage(filename + "-gaus.raw", TerraGodContext.Instance() 
         .Config.PpaCandidateRange); 
        gaussian.ConformRepeatPadding(); 
       } 
       else 
       { 
        gaussian = new PaddedImage(resolution, resolution, false, 
         TerraGodContext.Instance().Config.PpaCandidateRange); 
        PaddedImage temp = new PaddedImage(resolution, resolution, false, 
         TerraGodContext.Instance().Config.PpaCandidateRange); 
        ImageProcessing.CalcGaussian(image,gaussian,temp, 16f* resolution 
         /TerraGodContext.Instance().Config.ExmplResDS); 
       } 
      } 
     } 
     return gaussian; 
    } 

} 

Resharper дает мне три предупреждения:

  1. На гауссовой в gaussian.ConformRepeatPadding() он говорит возможна некорректная реализация Double - блокировка чеков. Прочитайте доступ к проверенному полю..
  2. На гауссовском языке в gaussian = new PaddedImage( говорится: Возможная некорректная реализация блокировки с двойной проверкой. Возможно множественный доступ на запись к отмеченному полю..
  3. На гауссовском языке в ImageProcessing.CalcGaussian(image,gaussian,temp, указано Возможная некорректная реализация блокировки с двойной проверкой. Прочитайте доступ к проверенному полю. (То же предупреждение, что и номер 1).

Могу ли я быть глупым или resharper?

PS: В случае, если я пропустил что-то важное, вот полный код класса с частями, которые я пропустил выше.

using System; 
using System.IO; 
using System.Text; 

namespace UPlus.TerrEngine 
{ 
internal class ExmplFile : IEquatable<ExmplFile> 
{ 
    private readonly string filename; 
    private int resolution; 
    private volatile PaddedImage image; 
    private volatile PaddedImage gaussian; 
    private object lockObject=new object(); 
    public ExmplFile(string abstractFileName, int res) 
    { 
     filename = new StringBuilder(abstractFileName).Append("-").Append(res.ToString("D4")).ToString(); 
     resolution = res; 
    } 

    internal FilledList<MatchItem>[] GetMatchItems(int groupIdx, int regionIdx) 
    { 
     AlgorithmConfig Config = TerraGodContext.Instance().Config; 
     if (resolution !=Config.ExmplResDS) throw new Exception(); 
     PpaGraph exmplGraph = new PpaGraph(Config.ExmplResDS, Config.ExmplResDS, Config.ExmplResDS 
      /Config.PpaExmplResDS); 
     exmplGraph.Calculate(Image(), true, false, DebugOpts.BranchMin); 
     exmplGraph.PrepareGraphForMatch(Config.ExmplCptRadius, Config.ExmplProcNodeDistance); 
     exmplGraph.CalcExmplProcNodeGroups(groupIdx); 
     MatchItemFinder matchItemFinder=new MatchItemFinder(Image(),Config.ExmplPatchSizeDS,true); 
     matchItemFinder.Init(exmplGraph.processNodesGroups, regionIdx); 
     return matchItemFinder.matchItems; 
    } 

    internal FastList<MatchItem> LoadMatchItems() 
    { 
     throw new Exception(); 
    } 

    internal PaddedImage Image() 
    { 
     if (image != null) 
     { 
      return image; 
     } 
     lock (lockObject) 
     { 
      if (image == null) 
      { 
       image=LoadImage(filename + ".raw", TerraGodContext.Instance().Config 
        .PpaCandidateRange); 
      } 
     } 
     return image; 
    } 
    internal PaddedImage Gaussian() 
    { 
     if (gaussian != null) 
     { 
      return gaussian; 
     } 
     lock (lockObject) 
     { 
      if (gaussian == null) 
      { 
       Image(); 
       if (File.Exists(filename + "-gaus.raw")) 
       { 
        gaussian = LoadImage(filename + "-gaus.raw", TerraGodContext.Instance() 
         .Config.PpaCandidateRange); 
        gaussian.ConformRepeatPadding(); 
       } 
       else 
       { 
        gaussian = new PaddedImage(resolution, resolution, false, 
         TerraGodContext.Instance().Config.PpaCandidateRange); 
        PaddedImage temp = new PaddedImage(resolution, resolution, false, 
         TerraGodContext.Instance().Config.PpaCandidateRange); 
        ImageProcessing.CalcGaussian(image,gaussian,temp, 16f* resolution 
         /TerraGodContext.Instance().Config.ExmplResDS); 
       } 
      } 
     } 
     return gaussian; 
    } 

    private PaddedImage LoadImage(string fileName, int padding) 
    { 
     PaddedImage img=new PaddedImage(resolution,resolution,false,padding); 
     img.LoadRaw(fileName); 
     img.ConformRepeatPadding(); 
     return img; 
    } 

    public bool Equals(ExmplFile other) 
    { 
     return filename == other.filename; 
    } 

    public override int GetHashCode() 
    { 
     return filename.GetHashCode(); 
    } 
    } 
} 

EDIT: Вот скриншот в случае ситуация не ясна:

enter image description here

+1

Это задокументировано [здесь] (https://www.jetbrains.com/help/resharper/2016.1/ReadAccessInDoubleCheckLocking.html) и [здесь] (https://www.jetbrains.com/help/resharper/2016.1 /PossibleMultipleWriteAccessInDoubleCheckLocking.html) – stuartd

ответ

2

В общем, в пределах lock вы должны использовать локальные переменные и сделать запись в поле с полем последним возможным действием.

В противном случае, что вы делаете с ним посередине? Например. какой ConformRepeatPadding() do? Я предполагаю, что он имеет побочные эффекты, так как он не имеет возвращаемого значения? Но к тому времени, как вы его назовете, у вас уже есть , выставив этот объект, присвоив его значение полю.

Другие темы могут наблюдать этот объект в «частично сконструированном» состоянии, если вы назначаете поле слишком рано, так как я говорю, вы должны, как правило, написать в поле последнее возможное действие внутри lock ,

+0

Да ConformRepeatPadding() изменяет состояние гауссова. Я не думал об этом так, потому что этот метод - единственный способ получить гауссовское поле. Благодаря вам теперь я понимаю причину, и это имеет смысл. –

+2

Да, локальный переменный подход действителен, но я думаю, что Lazy был разработан, чтобы сделать ошибки, подобные этому, менее вероятным –

2

Попробуйте определения вашей собственности с помощью Lazy<T> вместо этого. Ваш код выглядит правильно, но это должно избавиться от ошибки и является каркасной картиной для такого типа вещей.

+0

Хороший улов. Да, я сделаю это. –

0

С вашим замком что-то странное. Я alwais видел это на статическом объекте блокировки, а не на частном объекте. Если ваш класс не одинарный, ваши блокировки проходят через блокировку.

+1

Нет, это не синглтон, и я думаю, что использование объектов экземпляра в качестве монитора блокировки вполне допустимо. –

+0

yes, действует до тех пор, пока все параллельные потоки не будут использовать этот экземпляр для мониторинга согласованного ресурса. если экземпляр является зависящей от потока зависимостью переменной, она не разделяется, и поэтому нить не будет ждать какой-либо другой поток. –

Смежные вопросы