Changeset 42fc22e


Ignore:
Timestamp:
Sep 4, 2011 7:36:08 PM (8 years ago)
Author:
zzz <zzz@…>
Branches:
master
Children:
63958df
Parents:
b39ba06
Message:

Remove one global lock in OutboundMessageRegistry?.
This isn't the cause of the ISJ deadlocks though.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • router/java/src/net/i2p/router/transport/OutboundMessageRegistry.java

    rb39ba06 r42fc22e  
    2626import net.i2p.router.ReplyJob;
    2727import net.i2p.router.RouterContext;
     28import net.i2p.util.ConcurrentHashSet;
    2829import net.i2p.util.Log;
    2930import net.i2p.util.SimpleTimer;
     
    3839    /** map of active MessageSelector to either an OutNetMessage or a List of OutNetMessages causing it (for quick removal) */
    3940    private final Map<MessageSelector, Object> _selectorToMessage;
    40     /** set of active OutNetMessage (for quick removal and selector fetching) */
     41    /**
     42     *  set of active OutNetMessage (for quick removal and selector fetching)
     43     *  !! Really? seems only for dup detection in registerPending().
     44     *  Changed to concurrent, but it could perhaps be removed completely,
     45     *  It would seem difficult to add a dup since every OutNetMessage is different,
     46     *  and it's generally instantiated just before ctx.outNetMessagePool().add().
     47     *  But in TransportImpl.afterSend() it does requeue a previous ONM if allowRequeue=true.
     48     */
    4149    private final Set<OutNetMessage> _activeMessages;
    4250    private final CleanupTask _cleanupTask;
     
    4856        _selectors = new ArrayList(64);
    4957        _selectorToMessage = new HashMap(64);
    50         _activeMessages = new HashSet(64);
     58        _activeMessages = new ConcurrentHashSet(64);
    5159        _cleanupTask = new CleanupTask();
    5260    }
     
    6472        // Calling the fail job for every active message would
    6573        // be way too much at shutdown/restart, right?
    66         synchronized (_activeMessages) {
    67             _activeMessages.clear();
    68         }
     74        _activeMessages.clear();
    6975    }
    7076   
     
    8591     * This is called only by InNetMessagePool.
    8692     *
     93     * TODO this calls isMatch() in the selectors from inside the lock, which
     94     * can lead to deadlocks if the selector does too much in isMatch().
     95     * Remove the lock if possible.
     96     *
    8797     * @param message Payload received that may be a reply to something we sent
    8898     * @return non-null List of OutNetMessage describing messages that were waiting for
     
    98108            //    MessageSelector sel = iter.next();
    99109            for (int i = 0; i < _selectors.size(); i++) {
    100                 MessageSelector sel = (MessageSelector)_selectors.get(i);
     110                MessageSelector sel = _selectors.get(i);
    101111                boolean isMatch = sel.isMatch(message);
    102112                if (isMatch) {
     
    142152                if (removed) {
    143153                    if (msg != null) {
    144                         synchronized (_activeMessages) {
    145                             _activeMessages.remove(msg);
    146                         }
     154                        _activeMessages.remove(msg);
    147155                    } else if (msgs != null) {
    148                         synchronized (_activeMessages) {
    149                             _activeMessages.removeAll(msgs);
    150                         }
     156                        _activeMessages.removeAll(msgs);
    151157                    }
    152158                }
     
    196202        if (sel == null) throw new IllegalArgumentException("No reply selector?  wtf");
    197203
    198         boolean alreadyPending = false;
    199         synchronized (_activeMessages) {
    200             if (!_activeMessages.add(msg))
    201                 return; // dont add dups
    202         }
     204        if (!_activeMessages.add(msg))
     205            return; // dont add dups
     206
    203207        synchronized (_selectorToMessage) {
    204208            Object oldMsg = _selectorToMessage.put(sel, msg);
     
    247251        if (!stillActive)
    248252            synchronized (_selectors) { _selectors.remove(sel); }
    249         synchronized (_activeMessages) { _activeMessages.remove(msg); }
     253        _activeMessages.remove(msg);
    250254    }
    251255
     
    294298                    }
    295299                    if (msg != null) {
    296                         synchronized (_activeMessages) {
    297                             _activeMessages.remove(msg);
    298                         }
     300                        _activeMessages.remove(msg);
    299301                        Job fail = msg.getOnFailedReplyJob();
    300302                        if (fail != null)
    301303                            _context.jobQueue().addJob(fail);
    302304                    } else if (msgs != null) {
    303                         synchronized (_activeMessages) {
    304                             _activeMessages.removeAll(msgs);
    305                         }
     305                        _activeMessages.removeAll(msgs);
    306306                        for (OutNetMessage m : msgs) {
    307307                            Job fail = m.getOnFailedReplyJob();
Note: See TracChangeset for help on using the changeset viewer.