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 выглядит как-то экзотично. Ведь синхронизованный метод должен захватить монитор, а монитор прилагается к объекту, т.е. экземпляру класса, а не к классу как таковому.