Another deadlock, CHARACTER_EFF

[Arufonsu]

[asafgb]

can you show in your code:
in File:
src>client> maplecharacter>

(print from line 2650-2700)

[Arufonsu]

2630-2703 from maplecharacter:

    public void buffExpireTask() {
        if (buffExpireTask == null) {
            buffExpireTask = TimerManager.getInstance().register(new Runnable() {
                @Override
                public void run() {
                    Set<Entry<Integer, Long>> es;
                    List<MapleBuffStatValueHolder> toCancel = new ArrayList<>();
                    
                    effLock.lock();
                    chrLock.lock();
                    try {
                        es = new LinkedHashSet<>(buffExpires.entrySet());
                        
                        long curTime = Server.getInstance().getCurrentTime();
                        for(Entry<Integer, Long> bel : es) {
                            if(curTime >= bel.getValue()) {
                                toCancel.add(buffEffects.get(bel.getKey()).entrySet().iterator().next().getValue());    //rofl
                            }
                        }
                    } finally {
                        chrLock.unlock();
                        effLock.unlock();
                    }
                    
                    for(MapleBuffStatValueHolder mbsvh : toCancel) {
                        cancelEffect(mbsvh.effect, false, mbsvh.startTime);
                    }
                }
            }, 1500);
        }
    }
    
    public void cancelSkillCooldownTask() {
        if (skillCooldownTask != null) {
            skillCooldownTask.cancel(false);
            skillCooldownTask = null;
        }
    }

    public void skillCooldownTask() {
        if (skillCooldownTask == null) {
            skillCooldownTask = TimerManager.getInstance().register(new Runnable() {
                @Override
                public void run() {
                    Set<Entry<Integer, MapleCoolDownValueHolder>> es;
                    
                    effLock.lock();
                    chrLock.lock();
                    try {
                        es = new LinkedHashSet<>(coolDowns.entrySet());
                    } finally {
                        chrLock.unlock();
                        effLock.unlock();
                    }
                    
                    long curTime = System.currentTimeMillis();
                    for(Entry<Integer, MapleCoolDownValueHolder> bel : es) {
                        MapleCoolDownValueHolder mcdvh = bel.getValue();
                        if(curTime >= mcdvh.startTime + mcdvh.length) {
                            removeCooldown(mcdvh.skillId);
                            client.announce(MaplePacketCreator.skillCooldown(mcdvh.skillId, 0));
                        }
                    }
                }
            }, 1500);
        }
    }
    
    public void cancelExpirationTask() {
        if (itemExpireTask != null) {
            itemExpireTask.cancel(false);
            itemExpireTask = null;
        }
    }

[asafgb]

Just wonder
in which case it happen
you did some skill and then logout? maybe

[asafgb]

I tell you what i think that happen:
you use some skill with cooldown
and then there skillCooldownTask that runnable at any time
there function that called: "cancelSkillCooldownTask"
that cansel that Task runnable,
even if the CharLock is locked
and the cancelSkillCooldownTask is run when

1. you change channel

2. you enter to CashShop

but there 0.0001% that really happen because you need to have a lot Skill in cooldown that
es = new LinkedHashSet<>(coolDowns.entrySet());
still didnt finish and while it still running
you did one of thing i said before
that can create DeadLock
@ronancpl what you think?

[ronancpl]

I think this:

Somewhere around the server execution task, chrLock (or something else) is being locked before effLock. The locking chain pattern always expects one lock type to be locked before another, in order, else a deadlock configures.

The pattern is: effLock always precedes chrLock. However, in some point on the code, chrLock or some other lock is preceding effLock. That's a fault, and will generate deadlock sometime.

[asafgb]

but the error on the server
return effLock as throw exeption

[resinate]

                effLock.lock();
                chrLock.lock();
                try {
                    es = new LinkedHashSet<>(coolDowns.entrySet());
                } finally {
                    chrLock.unlock();
                    effLock.unlock();
                }

whats the point of double locks in 1st place?

[ronancpl]

Some design decisions were made around effLocks that, since some effLock-protected code eventually executes chrLock locking, for whatever reason it is, now every effLock requires immediate chrLock as well to secure effLock-into-chrLock stability.