Java bean - synchronized method - кто тут прав?

[Sabina]

Не найдутся ли желающие разрешить наш спор?

На JSP классе мы пользуемся бином FormUtils и все методы этого бина syncronized static. В качестве аргумента им всем передается объект pageContext, иногда что-нить еще.

Вот пример одного из методов:

Code: Select all

public synchronized static String getAction(final String logoffAction,
final PageContext pageContext)
{
    final HttpServletRequest request =
(HttpServletRequest)pageContext.getRequest();
    final HttpSession session = pageContext.getSession();

    final String action = request.getParameter(ACTION);
    if ((logoffAction != null) // if suppied, check for logoff action
      && (action != null)  // skip if no action specified
      && action.equals(logoffAction)) // logoff
      session.invalidate();
    return action;
}

Мой одногруппник Джонни высказал такое предположение:

Have you looked at class FormUtil? All the methods in the class are
synchronized. This is beneficial when the state of an object (instance
variables) must be protected. Class FormUtil has no instance
variables, however. I am worried that we are serializing access to an
object which doesn't require special protection. I just can't see how
we gain anything. I can see how we lose performance, however.

А мне кажется что:

I believe the reason for those methods to be syncronized is to put a lock on
the pageContext object till the method is done. This is probably related to
the issue that JSP page can be used simultaneously by several users and
each gets a separate thread.

Кто из нас прав? А может оба неправы?

Заранее спасибо за ответ. Любопытно узнать истину.
Сабина

[Palych]

...Что-то не так в вашей системе...
Во-первых, согласно Вашему описанию, FormUtils не попадает под определение JavaBean.
Во-вторых, использование статических методов видится мне мягко говоря странным. JSP позволяет определить scope of the bean декларативно, а вы возлагаете это на сам класс... Проще говоря - методы не должны быть на static ни synchronized.
В третьих - synchronized method не защищает передаваемые данные. В вашем случае это защитит static variables.
Это равносильно:

Code: Select all

public static String getAction(final String logoffAction,
final PageContext pageContext)
{ 
  synchronized (FormUnit.class)
  {
.....
  }
}

[Chelya]

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

[Sabina]

...Что-то не так в вашей системе...

Ну тогда тем более хочется понять до конца.

Во-первых, согласно Вашему описанию, FormUtils не попадает под определение JavaBean.

Это один из классов входящих в состав универсального jar, которым мы пользуемся в классе. Препод его называет ...beans.jar Там всякие SQLUtils, GifEnсoder, класс для отправки емейлов и проч. Все что нужно для небольшого JSP приложения, которое делает базовые вещи: user self-registration, login, password reminder,etc.

Во-вторых, использование статических методов видится мне мягко говоря странным. JSP позволяет определить scope of the bean декларативно, а вы возлагаете это на сам класс... Проще говоря - методы не должны быть на static ни synchronized.
.

А как например декларативно?
В самом JSP обращение выглядит примерно так:

Code: Select all

String action = FormUtil.getAction(pageContext);
String sr_username = FormUtil.getValue(SR_USERNAME, pageContext);
String sr_password = FormUtil.getValue(SR_PASSWORD, pageContext);...

Как можно декларативно определить scope of the bean? Заранее прошу прощения если вопрос тупой, но все мы когда-то учимся.
Если долго объяснять, буду признательна хотя бы за линк.

В третьих - synchronized method не защищает передаваемые данные. В вашем случае это защитит static variables.

static variables чего? FormUtil?
Сам класс выглядит так:

Code: Select all

public class FormUtil
{
  // for use with self-processing forms
  // form example:
  // <form action='<%=response.encodeURL(request.getRequestURI())%>' method='post'>
  //  <input type='hidden' name='<%=FormUtil.ACTION%>' value='saveToDatabase'>
  // hyperlink example:
  //  <a href='<%=response.encodeURL(request.getRequestURI())%>?<%=FormUtil.ACTION%>=remindMe'>click here</a>
  //
  public final static String ACTION = "action";

  private final static String REFERER = "referer"; 
  private FormUtil(){} // uninstantiable class
......[методы]
}

Получается Джонни прав, что метод тут синхронизировать не надо?

Это равносильно:

Code: Select all

public static String getAction(final String logoffAction,
final PageContext pageContext)
{ 
  synchronized (FormUnit.class)
  {
.....
  }
}

Понятно, что synchronized я поняла совсем неверно. А какой может быть реальный пример, когда synchronized (PRIMER.class) имеет смысл. Какие например static variables (вернее variables ЧЕГО) нужно защищать? Желательно на примере той же JSP, если возможно.

Спасибо,
Сабина

[Sabina]

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

Понятно. А pageContext передается методу getValue как final, чтобы его там ненароком не изменили?

Сабина

[JustMax]

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

Понятно. А pageContext передается методу getValue как final, чтобы его там ненароком не изменили?

Сабина

К сожалению в Java описание параметра как final не подразумевает невозможность изменения аттрибутов обьекта. Всего-лишь запрещает присваивать параметру ссылку на другой обьект. Final в java вообще имеет мало смысла для не простых и не nonmutable типов. А по поводу синхронизирования методов и потоков - вам нужно обьяснение в принципе - а не частный случай в JSP (скажем BruceEckel http://64.78.49.204/ ). Как раз J2EE и призвана избежать использования потоков, синхронизации и т.д. мало того в J2EE даже и не рекомендуется(запрещается) это делать дабы не попасть в конфликт с J2EE контейнером.

[Sabina]

А по поводу синхронизирования методов и потоков - вам нужно обьяснение в принципе - а не частный случай в JSP (скажем BruceEckel http://64.78.49.204/ ). Как раз J2EE и призвана избежать использования потоков, синхронизации и т.д. мало того в J2EE даже и не рекомендуется(запрещается) это делать дабы не попасть в конфликт с J2EE контейнером.

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

Сабина

[JustMax]

Общий смысл :

http://java.sun.com/j2ee/j2ee-1_4-fr-spec.pdf

стр 48-49 86-87,

Конкретика в спецификациях по JSP и EJB. Конкретное место указать не могу - надо поискать.

[testuser]

Вообще сервлет и, соответсвенно, JSP может использовать два подхода для обработки одновременных запросов к одному и тому же ресурсу.
1. Single threaded - тогда для каждого запроса создается новый экземпляр Servlet, который обрабатывает запрос. Тогда заботиться о синхронизации не нужно (ну, почти не нужно ), но страдает производительность.
2. Multithreaded - создается один экземпляр сервлета, который обрабатывает запросы одновременно. Но для этого shared resources нужно синхронизировать.

Тут, как я понял shared resource - pageContext. Нужно аккуратно следить за тем, чтобы его изменение не повлияло на другие запросы, возможно исполняющиеся параллельно.

[Sabina]

http://java.sun.com/j2ee/j2ee-1_4-fr-spec.pdf

Мне кажется это я сама отошла от сути исходного вопроса, заведя разговор о beans, multiuser environment, etc. Попробую задать вопрос снова.

У нас имеется класс FormUtil, который мы вызываем для работы с JSP pageContext, и в котором есть следующие методы:

// CONTENTS:
// public synchronized static String getAction(PageContext)
// public synchronized static String getAction(String, PageContext)
// public synchronized static boolean needToLoginFirst(String, String, PageContext)
// public synchronized static boolean redirectToReferer(PageContext)
// public synchronized static boolean redirect(String, PageContext)
// public synchronized static String getValue(String, PageContext)
// public synchronized static String getValue(String, String, PageContext)
// public synchronized static String getValue(String, String, String, PageContext)
// public synchronized static boolean getValue(String, boolean, String, PageContext)
// public synchronized static String apo(String)
// public synchronized static String select(String, String, String[])
// public synchronized static String select(String, String, String[], String[])
// public synchronized static String checkbox(String, boolean)
// public synchronized static String radio(String, String, String)
// public synchronized static boolean checkDuplicateEmail(String, String)
// public synchronized static boolean checkEmailFormat(String)
// public synchronized static String getCookie(String, PageContext)
// public synchronized static boolean hasCookie(String, PageContext)
// public synchronized static void setCookie(String, String, int, PageContext)
// public synchronized static void setCookie(String, String, PageContext)
// public synchronized static void removeCookie(String, PageContext)

Как вы думаете какие мотивы двигали автором этого класса, когда он деклалировал эти методы synchronized & static? Если это неверно и потенциально опасно, то прошу привести практический пример чем плохим это может кончится.

Спасибо,
Сабина

[testuser]

Вообще static не имеет отношения к доступу. Static просто означает, что для вызова такого метода не нужно иметь экземпляр этого класса. Для разнообразных утилит это бывает оправданно. В static можно пользоваться только статическими переменными либо переданными параметрами.
Насчет статических переменных нужно аккуратно смотреть чтобы не было одновременного неправильного использования.

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

Какие потенциальные проблемы могут быть - если вы делаете какое-то действие на pageContext, вы должны понимать что этот объект может быть изменен в другом методе. Например session.invalidate() или еще что-то.
Если в другом методе используется тот же pageContext, получится что посреди метода сессия изменилась. Чего в принципе не должно было быть.
Поэтому программу нужно писать рассчитывая, что это может произойти.

Final тут вообще не особо поможет - методы то все равно можно вызывать, а они могу изменить состояние объекта.

[JustMax]

На самом деле синхронизация здесь вообще не нужна т.к. вы в данном случае не работаете с переменными обьекта FormUtil. А при передаче параметров в метод (вообще не важно static или нет) происходит их копирование в локальный стек потока. PageContext и так создаются для каждого пользователя/jsp свои уникально, в зависимости от scope т.е. request, page, session. Только в одном случае синхронизация оправдана - на работу с обьектами сохраненными на уровне Application. Но и в этом случае синхронизовать надо бы не методы FormUtil а методы тех классов которые хранятся на уровне Application.

[Sabina]

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

Какие потенциальные проблемы могут быть - если вы делаете какое-то действие на pageContext, вы должны понимать что этот объект может быть изменен в другом методе. Например session.invalidate() или еще что-то.
Если в другом методе используется тот же pageContext, получится что посреди метода сессия изменилась. Чего в принципе не должно было быть.
Поэтому программу нужно писать рассчитывая, что это может произойти.

Спасибо за разъяснения, testuser. То есть отвечая на мой вопрос

- static метод объявлен для того, чтобы при каждом обращении к методам этого класса не нужно было создавать отдельную instance этого класса.

- synchronized, потому что в случае, если метод будет вызываться несколькими тредами одновременно, им придется встать в очередь.

То есть оба эти определения методов static и synchronized имеют право на существование и there is nothing wrong about declaring FormUtil methods this way.

- по-прежнему существует потенциальная проблема того, что pageContext может быть изменен другим методом еще до того, как обработка методом FormUtil завершена. И при этом передача pageContext как final - не помогает. То есть нужно предпринять еще какие-то шаги, чтобы предотвратить эту проблему.

Правильно я поняла в этот раз?

А кстати что случиться, если session.invalidate() например изменит pageContext, до того как отработает метод getAction?

Сабина

[Sabina]

На самом деле синхронизация здесь вообще не нужна т.к. вы в данном случае не работаете с переменными обьекта FormUtil.

Так в чем же тогда суть синхронизации? Как факт того, что метод может вызываться только одним тредом at a time соотноситься с тем, работаю я или не работаю с переменными объекта Form Util?

А при передаче параметров в метод (вообще не важно static или нет) происходит их копирование в локальный стек потока. PageContext и так создаются для каждого пользователя/jsp свои уникально, в зависимости от scope т.е. request, page, session.

То есть получается, что опасения по поводу того что, например, session.invalidate() может изменить pageContext напрасны, ибо метод Form Util все равно работает с копией в локальном стеке?

Только в одном случае синхронизация оправдана - на работу с обьектами сохраненными на уровне Application. Но и в этом случае синхронизовать надо бы не методы FormUtil а методы тех классов которые хранятся на уровне Application.

Это как-то идет в разрез с тем, что testuser объяснил. Получается что ситуация когда несколько thread-ов одновременно будут вызывать метод FormUtil либо непринципиальна либо невозможна?

Сабина

[lxf]

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

При этом другие потоки не только не могут выполнять код этого, но и любого другого синхронизованного метода. Т.е. пока поток выполняет один синхронизованный метод, все другие потоки сидят и ждут очереди выполнить свои синхронизованные методы. Вообще synchronized static выглядит как-то экзотично. Ведь синхронизованный метод должен захватить монитор, а монитор прилагается к объекту, т.е. экземпляру класса, а не к классу как таковому.

[Sabina]

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

Какой монитор вы имеете в виду?
JSP - на сервере, FormUtil, методы которого она вызывает тоже на сервере.

Сабина

[Chelya]

Sabina,

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

2. synchronized в данном классе не нужен, так как доступ в PageContext и HttpSession не надо синхронизировать.

3. Данный класс является наглядным примером того как не надо писать программы. Начиная с того, что JSP не должно иметь business logic и заканчивая тем, что static functions в большинстве случаев - зло. Static functions "break" OOP design (like global variables), делает maintanance сложнее, extensibility хуже.

[Sabina]

3. Данный класс является наглядным примером того как не надо писать программы. Начиная с того, что JSP не должно иметь business logic и заканчивая тем...

То есть вызов FormUtil из JSP можно назвать "JSP имеет business logic"?
А где обычно храниться business logic? В данный момент FormUtil входит в состав WEB-INF/lib/...beans.jar

[Amayan]

...static functions в большинстве случаев - зло. Static functions "break" OOP design (like global variables), делает maintanance сложнее, extensibility хуже.

Пожалуйста, объясните подробнее, почему Вы считаете, что static methods в большинстве случаев зло. По возможности аппелируя к личному опыту, а не к книгам, которые Вы прочитали про паттерны в Java или J2EE. Для меня будет весьма ценно, если вы сможете подкрепить утверждения "maintanance сложнее, extensibility хуже" описаниями конкретных проблем, с которыми Вам довелось столкнутся.
Я это все спрашиваю не из-за того, что собираюсь "зацепить" Вас. Я сравнительно недавно занимаюсь Java, но у меня уже сложилось впечатление, что большинство Java-программистов необоснованно избегают static methods, хотя стандартная библиотека Java использует их вовсю. Как например и Jakarta Commons.

[Dedal]

Как раз J2EE и призвана избежать использования потоков, синхронизации и т.д. мало того в J2EE даже и не рекомендуется(запрещается) это делать дабы не попасть в конфликт с J2EE контейнером.

EJB != J2EE

- synchronized, потому что в случае, если метод будет вызываться несколькими тредами одновременно, им придется встать в очередь.

да, synchronized для того, чтобы поставить методы в очередь (сериализовать).

На самом деле синхронизация здесь вообще не нужна т.к. вы в данном случае не работаете с переменными обьекта FormUtil.

Так в чем же тогда суть синхронизации? Как факт того, что метод может вызываться только одним тредом at a time соотноситься с тем, работаю я или не работаю с переменными объекта Form Util?Сабина

Цель synchronized -- сериализовать выполнение участка кода. Можно сериализовать кусок кода экземпляра объекта, целый метод экземпляра объекта или метод класса. Если Вы сериализуете метод экземпляра, то выполнение любых других методов этого экземпляра приостанавливается во время выполнения этого метода одним потоком. Таким образом, этот поток имеет в своем полном распоряжении все поля объекта, другим потокам изменение полей недоступно.

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

Какой монитор вы имеете в виду?
JSP - на сервере, FormUtil, методы которого она вызывает тоже на сервере.

Сабина

Монитор объекта, который Вы пытаетесь блокировать с помощью synchronized. Вам нужно почитать про потоки и ознакомиться хотя бы с базовыми принципами и терминологией. Монитор к классу тоже прилагается, разумеется. Хотя я не знаю, как static synchronized будет работать в случае нескольких класслоадеров. Надо почитать...

2. synchronized в данном классе не нужен, так как доступ в PageContext и HttpSession не надо синхронизировать.

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

3. Данный класс является наглядным примером того как не надо писать программы. Начиная с того, что JSP не должно иметь business logic и заканчивая тем, что static functions в большинстве случаев - зло. Static functions "break" OOP design (like global variables), делает maintanance сложнее, extensibility хуже.

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

[testuser]

Что значит static методы это зло? Их просто нужно применять в тех случаях, когда их нужно применять.
Если метод не зависит от конкретного экземпляра класса (объекта), тогда его нужно делать static. Делать их не статическими просто не имеет смысла, так как нужно будет создать объект только для того чтобы произвести какое-то действие, свойственное классу.
К примеру в классе Integer есть статический метод parceInt(String s). Было бы глупо создавать новый объект только для того, чтобы пропарсить строку - для таких целей есть статические методы.

[JustMax]

EJB != J2EE

. При чем тут EJB ? Вы давно спецификацию J2EE перечитывали ? A также спецификации всех технологий входящих в J2EE и общие принципы на которых они базируются ?

Цель synchronized -- сериализовать выполнение участка кода. Можно сериализовать кусок кода экземпляра объекта, целый метод экземпляра объекта или метод класса. Если Вы сериализуете метод экземпляра, то выполнение любых других методов этого экземпляра приостанавливается во время выполнения этого метода одним потоком. Таким образом, этот поток имеет в своем полном распоряжении все поля объекта, другим потокам изменение полей недоступно.

Одно маленькое уточнение - ето никак не поможет при попытке изменения обьекта, ссылка на который передается как параметер в synchronized метод. Он с такой же легкостью будет изменен в то же самое время методами других классов&потоков.
Единственное спасение - synchronized методы, или если ето чужой или final класс, обеспечить изменения такого обьекта исключительно через унаследованный класс или класс wrapper, но уже с synchronized методами.

[vlad12345]

Насколько я помню JSP/Servlets спецификацию, конкретный pageContext объект может быть доступен только одному thread-у (по окончании thread-а может быть reused, но параллельный доступ исключен), так что синхронизация в данном случае ни к чему.

В принципе такой вот FormUnit в теории мог бы иметь смысл, если бы предполагалось через него также обращаться и к session, application объектам и другим, действительно требующим синхронизации. Только тогда ВСЕ обращения должны делаться ТОЛЬКО через FormUnit методы (светофоры были бы бесполезны если бы не все водители соблюдали правила) -- этакий распорядитель строго следящий за очередью. Вот только скорее всего это оказалось бы узким местом в аппликации, представьте, скажем, что в зале работает десять касс, но распорядитель следит , чтобы в зале мог находиться только один человек, а остальные ждут в очереди за дверью.

Еще мне кажется, что на FormUnit как-то слишком много всего возложено, например при запросе getAction заодно может сработать logoff - session.invalidate(), когда-нибудь такой побочный эффект может оказатья неприятной неожиданностью. И все эти synchronized и final скорее всего были сделаны для универсализма и единообразия. Кстати final могло бы пригодиться, если бы внутри методов применялись inner classes, работющие с параметрами метода, т.е. из соображений универсальности -- пусть все будет final, где-нибудь пригодится ...

[vlad12345]

2. synchronized в данном классе не нужен, так как доступ в PageContext и HttpSession не надо синхронизировать.

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

Imho, с Session действительно надо поосторожнее: пользователь может открыть в броузере несколько окон, или еще проще -- несколько фреймов.

[vlad12345]

del... sorry, не в тему