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.