LINUX.ORG.RU

Java: synchronize(string.intern()) {

 


0

5

Сабж, нашел тут: https://github.com/gocd/gocd/blob/master/util/src/com/thoughtworks/go/util/Dy...

По моему так делать нельзя, а как вы думаете (и почему, т.к. я привести доказательства порочности такой практики не могу с ходу)?

ps. Судя по всему народ вдохновляется сим:

https://justavo.wordpress.com/2004/09/18/stringintern-and-synchronization/

особенно доказательство верности сего хака внушает:

I used this aproximation about three years ago on an entry point of a online e-banking application, and worked quite well.

Deleted

Почему нельзя? Мне кажется, можно. Я такой же код писал для синхронизации по сессии:

synchronize (session.getId().intern()) { ... }

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

Legioner ★★★★★ ()
Последнее исправление: Legioner (всего исправлений: 3)
Ответ на: комментарий от Legioner

Почему нельзя?

В старых версиях jre куча способов загадить память этим. В новых у тебя будет лютая гигантская таблица из рандомных строк (ну в твоем случае).

При том что для сессий - ты можешь прямо synchronize (session) - писать, зачем извращаться не ясно?

А для общего случая есть «пул блокировок».

Deleted ()

не думаю что этот код корректен вообще. Если дергать getLock из разных потоков со случайными ключами то можно развалить hash таблицу с локами, которая ниразу не concurrent.

Intern плох тем что его можно загадить большим объемом строк, он на это не рассчитан и довольно неохотно обслуживается GC. И еще не известно кто им пользуется, для блокировок лучше использовать специально созданные локи, а не переиспользовать доступные из-вне объекты.

Еще момент в том что сам метод intern не особо производителен при конкурентной нагрузке, там за ним тоже стоит хеш-таблица с блокировками.

maxcom ★★★★★ ()
Ответ на: комментарий от maxcom

Если дергать getLock из разных потоков со случайными ключами то можно развалить hash таблицу с локами, которая ниразу не concurrent.

Да, самое главное и очевидное я не заметил.

Deleted ()
Последнее исправление: Deleted (всего исправлений: 1)

Да, так явно делать нельзя.

Хотя у них много перлов:
1) Самодельный интерфейс Procedure, когда можно было бы просто использовать Runnable
2) Самописный парсинг CSV (https://github.com/gocd/gocd/blob/master/util/src/com/thoughtworks/go/util/Cs...). Неправильный. Тупо сплитят по запятой. Явно не по RFC (например, double-quotes вообще игнорятся).
...
да много всего там.

kovrik ★★★★★ ()

Там на самом деле неправильно работают с HashMap:

Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more mappings; merely changing the value associated with a key that an instance already contains is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the map. If no such object exists, the map should be «wrapped» using the Collections.synchronizedMap method. This is best done at creation time, to prevent accidental unsynchronized access to the map

Чтобы все круто было нужно было использовать ConcurrentHashMap с putIfAbsent

borisych ★★★★★ ()
Ответ на: комментарий от Deleted

В старых версиях jre куча способов загадить память этим. В новых у тебя будет лютая гигантская таблица из рандомных строк (ну в твоем случае).

Ну я про более менее новые. GC собирает неиспользуемые интернированные строки, так что проблем с этим нет.

При том что для сессий - ты можешь прямо synchronize (session) - писать, зачем извращаться не ясно?

Не могу, объект сессии не обязательно одинаковый для разных запросов. Я бы сказал — не просто не обязательно одинаковый, а гарантированно разный, если какой-нибудь хитрый фильтр его враппирует.

Legioner ★★★★★ ()
Ответ на: комментарий от Legioner

Ну я про более менее новые. GC собирает неиспользуемые интернированные строки, так что проблем с этим нет.

IIRC начиная с java8 заинтерненые строки больше не в permgen, да и самого permgen больше нет

Не могу, объект сессии не обязательно одинаковый для разных запросов

а зачем тогда вообще нужен synchronized? что и от чего он защищает?

Возвращаясь к исходному вопросу, можно так делать, но зачем? во-первых, synchronized отлично заменяется более высокоуровневыми вещами из j.u.concurrent. Во-вторых, синхронизация нужна при одновременном доступе к какой-то неконкурентной структуре и в этом случае нужно синхронизироваться на специально созданных локах, связанных со структурой, но уж никак не на строках.

phoenix ★★★★ ()
Ответ на: комментарий от phoenix

а зачем тогда вообще нужен synchronized? что и от чего он защищает?

Он позволяет писать обработчики, которые меняют объекты в сессии без блокировок.

Legioner ★★★★★ ()
Ответ на: комментарий от Deleted

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

Legioner ★★★★★ ()

I used this aproximation about three years ago on an entry point of a online e-banking application, and worked quite well.

ынтырпрайз!

invy ★★★★★ ()
Ответ на: комментарий от Legioner

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

лок нужен чтобы защищать shared mutable state (при этих словах дрожь пробегает по всему телу). Нет SMS — нет необходимости в локах.

phoenix ★★★★ ()
Ответ на: комментарий от phoenix

Сессия это по определению хранилище shared mutable state. Если я пишу про лок для сессии, значит у меня есть этот shared mutable state.

Legioner ★★★★★ ()
Ответ на: комментарий от Legioner

Ээ суть в том что надо писать так:

 void doGet(...) {
   syncronize(sessionLock) {
     //get session vars
   }
   Thread.sleep(10h);//we do long task
   syncronize(sessionLock) {
     //set session vars
   }
 }

Но ни в коем случае не так:

 void doGet(...) {
   syncronize(sessionLock) {
     //get session vars
     Thread.sleep(10h);//we do long task
     //set session vars
   }
 }

У тебя судя по описанию последний случай и по сути сервер одному клиенту отвечает в «одном потоке», и ждет пока все отдуплятся даже если там надо взять ежика из кеша.

Deleted ()
Ответ на: комментарий от Legioner

не понял эту фразу, можешь развернуть как-нибудь, кусочек кода?

stevejobs ★★★★☆ ()
Ответ на: комментарий от stevejobs

Как-то так:

    void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) {
        HttpServletRequest httpRequest = (HttpServletRequest) request;
        HttpSession session = httpRequest.getSession(false);
        if (session != null) {
            synchronized (session.getId().intern()) {
                chain.doFilter(request, response);
            }
        } else {
            chain.doFilter(request, response);
        }
    }

Legioner ★★★★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.