Имеется таск. Нужно реализовать многопоточное приложение, без использования synchronized.
Есть порт с причалами(1 причал - 1 корабль). Есть корабли с грузом и склад. В причале корабль может перенести груз в склад либо на другой корабль.
Я пробовал просто в каждой функции ставить в начале и конце ReentrantLock. Видимо, я плохо понял как он работает. С synchronized можно просто навесить его на каждый трэд, который первым приходит в функцию и всё работает на ура, а без него - тупик. Может кто подсказать как же правильно использовать lock или другой способ реализации такого приложения?
GitHub
Кусок кода:
private ReentrantLock lock = new ReentrantLock();
public boolean tryTransfer(Ship ship) {// ф-ия пытается добавить груз(cargo) в хранилище
boolean flag; // capacity - объём хранилища
lock.lock();
if (capacity + ship.getCargo()>maxCapacity) {
flag = false;
} else
{
capacity+=ship.getCargo();
ship.setCargo(0);
flag = true;
}
lock.unlock();
return flag;
}
ReentrantLock
внутри методов и там же их используете (это видно в классах Dock
и Ship
). Это не имеет никакого смысла, если эти Lock'и вы не передаёте куда-либо ещё. Храните ReentrantLock
в полях.Ваша реализация Storage#tryTransfer
(та, что на GitHub, а не здесь) почти корректная. Вы правильно ограничиваете доступ к данным самого Storage
. Однако в этом методе вы неправильно работаете с Ship
- методы Ship#getCargo
и Ship#setCargo
не являются синхронизированными.
В данном случае вам нужно брать блокировку и в Storage
, и Ship
. Примерно так (по аналогии можете переделать и всё остальное):
public class Ship {
private final ReentrantLock lock = new ReentrantLock();
private double cargo;
public double getCargo() {
this.lock.lock();
try {
return this.cargo;
} finally {
this.lock.unlock();
}
}
public void setCargo(double cargo) {
this.lock.lock();
try {
this.cargo = cargo;
} finally {
this.lock.unlock();
}
}
public ReentrantLock getLock() {
return this.lock;
}
}
public class Storage {
private final ReentrantLock lock = new ReentrantLock();
private final double maxCapacity = 100;
private double capacity;
public boolean tryTransfer(Ship ship) {
this.lock.lock();
try {
ship.getLock().lock();
try {
if (this.capacity + ship.getCargo() > this.maxCapacity) {
return false;
}
this.capacity += ship.getCargo();
ship.setCargo(0);
} finally {
ship.getLock().unlock();
}
return true;
} catch (Exception e) {
e.printStackTrace();
} finally {
this.lock.unlock();
}
return false;
}
}
Зачем в пункте #2 в Storage#tryTransfer
я дополнительно брал блокировку над Ship#getLock
, если она уже была в Ship#getCargo
и Ship#setCargo
? Дело в том, что возможна ситуация, когда одновременно несколько потоков будут работать с одним и тем же экземпляром Ship
. Т.е. мы вызвали Ship#getCargo
и получили, например, 10. И ожидаем, что, когда мы вызовем Ship.setCargo(0)
, cargo всё ещё будет равен 10. Однако без дополнительной синхронизации между нашими вызовами Ship#getCargo
и Ship#setCargo
какой-нибудь другой поток может сам вызвать Ship#setCargo
с каким-нибудь другим значением, и получится, что в момент нашего вызова Ship#setCargo(0)
cargo равен не 10, как было изначально, а например, 12.
UPDATE
Ship#run
вы забываете взять блокировку у Dock
при работе с ним.Ship#run
вы вызываете ReentrantLock#lock
внутри try-finally
, а не до него. Так делать нельзя, так как есть риск, что try-finally
прервётся (например, из-за исключения) до взятия блокировки, а ReentrantLock#unlock
всё равно будет вызван.Ship#run
вы вызываете Dock#setCurrentShip
? Не лучше просто положить Ship
в Dock#shipsQueue
, а дальше причал сам разберётся, какой корабль обработать в первую очередь?Dock#run
после вызова ReentrantLock#lock
сразу делайте try-finally
. Исключение может произойти даже в обычном System.out.println()
, из-за чего ваш ReentrantLock#unlock
не будет вызван.Dock#run
вы сперва проверяете Dock#currentShip
и только потом берёте блокировку. И то не всегда. Если бы Dock#currentShip
не торчал наружу, то это было бы нормально, но ведь у вас есть Dock#getCurrentShip
и Dock#setCurrentShip
, что означает необходимость их синхронизации.Dock#setShipsQueue
. Не дело, когда кто угодно может взять и просто так подменить столь важную коллекцию. Лучше вообще её создавать внутри Dock
(а не принимать снаружи) и никому не отдавать - просто добавить публичные Dock#addShip
и Dock#removeShip
. Ну и Dock#isFree
можно модифицировать, чтобы он не значение какого-то поля возвращал, а проверял Dock#currentShip
и Dock#shipsQueue
(хотя тут уже решайте сами - ваша изначальная задумка мне неизвестна).Dock#isFree
всегда будет возвращать false
после первой обработки любого непустого корабля. Мне кажется, тут что-то не так - корабли когда-нибудь закончатся => причал должен в таком случае стать пустым, чего не происходит.UPDATE
ReentrantLock#unlock
не нужно вызывать ReentrantLock#tryLock
. Поток уже владеет блокировкой, так что такая проверка ничего не даст.Ship#run
у вас происходит бесконечное добавление корабля в очередь. Просто уберите цикл, он не нужен.Storage#run
совершенно некорректна. Даже если опустить пункт #1, вы зачем-то освобождаете блокировки перед return
и тому подобными операторами. В этом нет никакой необходимости - блок finally
выполняется всегда, даже после return
или continue
.Айфон мало держит заряд, разбираемся с проблемой вместе с AppLab
При использовании AssertassertEquals падает тест, из-за того что после листа expected добавляется пробел(непонятно почему)
При выполнение кода получаю ошибку, но причина мне не яснаПочему при создание банковской карты не находится user_id ведь я создаю через него,...
Я делаю небольшую игру - ретро-гонкуИ для управления автомобилем я решил использовать нажатия на экран