Как сделать метод эксклюзивным в многопоточном контексте?

StackOverflow https://stackoverflow.com/questions/148078

Вопрос

У меня есть метод, который должен выполняться исключительным образом.По сути, это многопоточное приложение, в котором метод периодически вызывается по таймеру, но также может запускаться вручную действием пользователя.

Давайте возьмем пример:

  1. Таймер истекает, поэтому метод называется.Задача может занять несколько секунд.

  2. Сразу после этого пользователь нажимает на некоторую кнопку, которая должна вызвать одну и ту же задачу:БАМ.Он ничего не делает, так как метод уже работает.

Я использовал следующее решение:

public void DoRecurentJob()
{
    if(!Monitor.TryEnter(this.lockObject))
    {
        return;
    }

    try
    {
        // Do work
    }
    finally 
    {
        Monitor.Exit(this.lockObject);
    }
}

Где lockObject объявляется так:

private readonly object lockObject = new object();

Редактировать :Будет только один экземпляр объекта, содержащего этот метод, поэтому я обновил объект блокировки, сделав его нестатическим.

Есть ли лучший способ сделать это?Или, может быть, это просто неправильно по какой-то причине?

Это было полезно?

Решение

Это выглядит разумно, если вы просто заинтересованы в том, чтобы метод не выполнялся параллельно.Ничто не мешает им запускаться сразу друг за другом, скажем, вы нажали кнопку через полмикросекунды после того, как таймер выполнил Monitor.Exit().

И наличие статического объекта блокировки, доступного только для чтения, также имеет смысл.

Другие советы

Вы также можете использовать Mutex или Semaphore если вы хотите, чтобы он работал в перекрестном процессе (с небольшим снижением производительности), или если вам нужно установить любое другое число, кроме одного из разрешенных одновременных потоков, выполняющих ваш фрагмент кода.

Существуют и другие сигнальные конструкции, которые могут работать, но ваш пример выглядит так, как будто он делает свое дело, причем в простой и понятной форме.

Незначительная гнида:если переменная lockObject является статической, то «this.lockObject» не должен компилироваться.Также кажется немного странным (и это должно быть, по крайней мере, тщательно задокументировано), что, хотя это метод экземпляра, он также имеет явно выраженное поведение в масштабе всего типа.Возможно, сделать это статическим методом, который принимает экземпляр в качестве параметра?

Действительно ли он использует данные экземпляра?Если нет, сделайте его статическим.Если это так, вам следует, по крайней мере, вернуть логическое значение, чтобы сказать, выполнили ли вы работу с экземпляром — мне трудно представить ситуацию, когда я хочу выполнить некоторую работу с конкретным фрагментом данных, но я не будьте осторожны, если эта работа не будет выполнена из-за того, что аналогичная работа выполнялась с другим фрагментом данных.

Я думаю, это должно сработать, но это немного странно.Я вообще не сторонник использования ручной блокировки только потому, что так легко ошибиться, но выглядит это нормально.(Вам необходимо учитывать асинхронные исключения между «if» и «try», но я подозреваю, что они не будут проблемой — я не могу вспомнить точные гарантии, предоставляемые CLR.)

Я думаю, Майкрософт рекомендует используя замок вместо прямого использования класса Monitor.Это обеспечивает более четкое расположение и гарантирует открытие замка при любых обстоятельствах.

public class MyClass
{

  // Used as a lock context
  private readonly object myLock = new object();

  public void DoSomeWork()
  {
    lock (myLock)
    {
      // Critical code section
    }
  }
}

Если вашему приложению требуется, чтобы блокировка охватывала все экземпляры MyClass, вы можете определить контекст блокировки как статическое поле:

private static readonly object myLock = new object();

Код в порядке, но я бы согласился с изменением метода на статический, поскольку он лучше передает намерение.Кажется странным, что между всеми экземплярами класса есть метод, который работает синхронно, но этот метод не является статическим.

Помните, что вы всегда можете сделать статический синхронный метод защищенным или закрытым, оставив его видимым только для экземпляров класса.

public class MyClass
{ 
    public void AccessResource()
    {
        OneAtATime(this);
    }

    private static void OneAtATime(MyClass instance) 
    { 
       if( !Monitor.TryEnter(lockObject) )
       // ...

Это хорошее решение, хотя статическая блокировка меня не очень устраивает.Прямо сейчас вы не ждете блокировки, поэтому у вас не возникнет проблем с взаимоблокировками.Но слишком заметные блокировки могут легко привести к неприятностям в следующий раз, когда вам придется редактировать этот код.Кроме того, это не очень масштабируемое решение.

Обычно я стараюсь сделать все ресурсы, которые пытаюсь защитить, от доступа нескольких потоков к частным переменным экземпляра класса, а затем также заблокировать их как частную переменную экземпляра.Таким образом, вы можете создать экземпляры нескольких объектов, если вам нужно масштабировать.

Более декларативный способ сделать это — использовать MethodImplOptions.Synchronized указатель метода, к которому вы хотите синхронизировать доступ:

[MethodImpl(MethodImplOptions.Synchronized)] 
public void OneAtATime() { }

Однако этот метод не рекомендуется по нескольким причинам, большинство из которых можно найти здесь и здесь.Я публикую это, чтобы у вас не возникло соблазна его использовать.На Яве, synchronized — это ключевое слово, поэтому оно может возникнуть при просмотре шаблонов потоков.

У нас есть аналогичное требование, с добавленным требованием, что если длительный процесс запрашивается снова, он должен поставиться в очередь для выполнения другого цикла после завершения текущего цикла.Это похоже на это:

https://codereview.stackexchange.com/questions/16150/singleton-task-running-using-tasks-await-peer-review-challenge

private queued = false;
private running = false;
private object thislock = new object();

void Enqueue() {
    queued = true;
    while (Dequeue()) {
        try {
            // do work
        } finally {
            running = false;
        }
    }
}

bool Dequeue() {
    lock (thislock) {
        if (running || !queued) {
            return false;
        }
        else
        {
            queued = false;
            running = true;
            return true;
        }
    }
}
Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top