Java: synchronize(string.intern()) {

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Aravind & Praveen

Индусы, Сэр!

Там на самом деле неправильно работают с 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

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

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

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

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

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

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

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

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

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

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

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

synchronized ... без блокировок.

?

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

Уу как все запущено.

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

ынтырпрайз!

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

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

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

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

 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
   }
 }

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

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

Как-то так:

    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);
        }
    }