SyncronizingProxyFactory: Pattern or antipattern?

Discussion in 'Java' started by Daniel Pitts, Mar 25, 2007.

  1. Daniel Pitts

    Daniel Pitts Guest

    I'm creating several interfaces (which actually may be remote). The
    implementations need to perform several operations "atomically" on one
    target object.
    I thought that it might be annoying to write stuff like:
    synchronized(target) { target.doOp(); target.doOp2(); }
    over and over again. So I thought to borrow a concept from Aspect
    Oriented Programming, and wrote myself a proxy factory.

    If my interfaces extend java.rmi.Remote, does this preclude me from
    using this Proxy class?
    <sscce>
    import java.lang.reflect.Proxy;
    import java.lang.reflect.InvocationHandler;
    import java.lang.reflect.Method;
    import java.lang.reflect.InvocationTargetException;

    /**
    */
    public class SynchronizingProxyFactory {
    private static class SynchronizedInvocationHandler<E> implements
    InvocationHandler {
    private final E target;
    private final Object sync;

    public SynchronizedInvocationHandler(E target, Object sync) {
    this.target = target;
    this.sync = sync;
    }

    public Object invoke(Object proxy, Method method, Object[] args)
    throws Throwable{
    synchronized(sync) {
    try {
    return method.invoke(target, args);
    } catch (InvocationTargetException e) {
    throw e.getTargetException();
    }
    }
    }
    }
    public static <E> E synchronizedProxy(Class<E> iface, E target,
    Object sync) {
    return (E)Proxy.newProxyInstance(iface.getClassLoader(),
    new Class[]{iface}, new
    SynchronizedInvocationHandler<E>(target, sync)) ;
    }
    }
    </sscce>

    Thanks,
    Daniel.
     
    Daniel Pitts, Mar 25, 2007
    #1
    1. Advertisements

  2. Daniel Pitts

    Chris Uppal Guest

    I worry a little when concepts like automatic synchronising wrappers turn up.
    The problem is that the appropriate granularity of synchronisation depends on
    the meaning of the operations invoked by the API, and by their interrelations,
    rather than by the boundaries between the actual methods in the API.

    If the API is unusually well-defined, then each semantic operation will
    correspond to exactly one method call (which is a valuable property for an API
    in itself, and even more so when API calls involve moving data over a
    network[*]). But if not -- as is typical -- then automatically adding
    "synchronize" to each method call will not provide the necessary safety.

    In your example:

    synchronized(target) { target.doOp(); target.doOp2(); }

    would be implemented by the automatic wrapper as if the code said:

    synchronized(target) { target.doOp(); }
    synchronized(target) { target.doOp2(); }

    which doesn't provide the same semantic guarantees at all.

    The converse of the same point is that the locking behaviour of an application
    is not something that can be fully encapsulated and hidden away. It is an
    issue that can have global ramifications for the design of an application,
    unless you want to risk poor performance or even deadlocks. In non threaded
    code you can (putting things in OO terms) just ask some object to do something,
    and let it worry about the implementation -- so the large-scale application
    architecture is not dependent on the details of how the object implements that
    operation -- but that is doesn't always apply where locks are concerned, since
    any held lock can potentially lock out any other piece of code. That's not
    always a problem, but where it is, I'd worry that low-level, automated
    facilities would not be "intelligent" enough to do the job properly.


    The greatest danger (IMO) is that the code which uses the wrapper /may/ be
    correctly designed to use synchronisation properly, or it may not, and there is
    no way to tell which just by reading it. I.e. there is no way to tell
    easily -- or even make a good guess -- whether the implicit synchronisation is
    any of:
    + correct by carefully considered design
    + correct by unconsidered default
    + correct but unnecessarily slow
    + incorrect

    That worry is greater still when there may be more than one proxy for the same
    object. In normal circumstances, multiple proxies are either a complete no-no
    (which may apply in your case, though you don't mention it); or are considered
    to be a normal state of affairs. If your semantic safeness depends on the
    proxies all agreeing on how they synchronise, then you may have to put more
    work into that part of the design than you've shown so far (presumably the
    separation of the 'target' and 'sync' objects in the wrapper API is intended to
    be a step in that direction).

    The only case I can think of (so far) where the automatic synchronised wrappers
    might be worthwhile is if the wrapper is /only/ intended to protect the state
    of the network proxy object itself, not the state of the object it is a proxy
    /for/. (In which case method-level boundaries would be appropriate -- but then
    I'd sort of hope the RMI implementation would have that under control anyway).

    -- chris
     
    Chris Uppal, Mar 25, 2007
    #2
    1. Advertisements

  3. Daniel Pitts

    Daniel Pitts Guest

    All of those are good points.

    A little more background for my particular project might help.

    I have a class "Game", which contains at least one member of type
    "GameState". GameState is an abstract class, implementing the "State/
    Strategy" pattern. Certain method calls are only valid with certain
    states. The actual value of the GameState object depends on the valid
    operations on Game.

    So, here comes the synchronizing design...

    Operations on Game need to be atomic, and Game is already large enough
    that I'd prefer not to delegate to GameState within game. I created a
    facade class "GameController" which references a "Game" object. There
    can be several GameController objects per Game object. GameController
    delegates to a few methods in Game, and then all methods in
    game.getGameState(). I am asserting that each method on
    GameController needs to be atomic to the given Game object. Hence the
    target/sync in my proxy.

    My target would the GameController, and my sync would be the Game.
    This allows the GameController to behave correctly even if there are
    more than one acting on the same Game.

    I realized that my code would have to be constructed with the concept
    that the method call itself was the atomic operation, not a group of
    them. This is knowledge that is important anyway, so if someone
    doesn't understand this, then they are likely to write incorrect code
    with and without my proxy, I think.

    To put it another way. my GameController only exposes methods which
    are transactional. It is also only invoked by a user performing an
    action in a GUI. The operations themselves are fast enough, and
    spread far enough apart, that the synchronization isn't going to harm
    performance, and is actually required for proper operation.


    Actually, I think you've got this backwards...
    The GameController would be proxied to a threadSafeGame. The
    threadSafeGame object stays on the server side, and then a
    gameControllerRemoteStub is sent to the Client side... The Stub is
    actually a proxy for the threadSafeGame, and the threadSafeGame a
    proxy for the gameController. Calls on the client side may be serial,
    but they might be parallel as well. the threadSafeGame makes them all
    serial with regards to the game object.

    Does this design make more sense? Or do you still think its an
    antipattern?
     
    Daniel Pitts, Mar 25, 2007
    #3
  4. Daniel Pitts

    Chris Uppal Guest

    OK. That makes sense.

    If GameController is talking directly to the GameState then your design is not,
    after all, a variant on the State pattern (or Strategy, come to that), and I no
    longer feel I understand what the design is. That's not to suggest that
    there's anything wrong with the design, but take my remaining comments with
    corresponding caution.

    It sounds as if you GameController has responsibility for grouping primitive
    operations on Game into semantically/transactionally atomic units. If so, then
    I question whether it's appropriate to hide that -- critical -- aspect of its
    function in the implicit synchronisation provided by a SynchronisingWrapper
    /around/ a GameController.

    Let me put it this way: I'm /less/ skeptical now ;-)

    -- chris
     
    Chris Uppal, Mar 27, 2007
    #4
  5. Daniel Pitts

    Esmond Pitt Guest

    Is there a reason why you don't just make all the GameController methods
    synchronized? as they have to be atomic?
     
    Esmond Pitt, Mar 27, 2007
    #5
  6. Daniel Pitts

    Chris Uppal Guest

    AIUI, they have to be atomic with respect to their common Game object, so just
    tagging their methods' declarations with 'synchronized' wouldn't do the trick.

    -- chris
     
    Chris Uppal, Mar 27, 2007
    #6
  7. Daniel Pitts

    Daniel Pitts Guest

    :)

    Okay, let me try to clarify the design further...

    The Game object contains a method GameState getState().

    GameControllerImpl has a reference to a Game object. each method on
    GameController either calls a method on Game, or a method on
    game.getState(). Calling, say, game.getState().deal() CAN change the
    current GameState object referenced in Game. So, by synchronizing
    against the Game object, I protect the game's state, as well as the
    game's gameState reference.

    The GameController interface is ALSO used as an RMI interface/GUI
    facade.

    Thats why I say I use the State pattern. The behaviour of the object
    returned by game.getState() is likely different every time.
     
    Daniel Pitts, Mar 27, 2007
    #7
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.