[MGNLSLOCK-11] Locks are not released Created: 08/Jul/11  Updated: 07/Sep/11  Resolved: 01/Sep/11

Status: Closed
Project: Magnolia Soft Locking Module
Component/s: None
Affects Version/s: 1.0.1
Fix Version/s: 1.0.2

Type: Bug Priority: Major
Reporter: Marc Werlen, Netcetera Assignee: Federico Grilli
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Enterprise Edition 4.4.2


Attachments: PNG File stakanovist-antti.png    
Issue Links:
relation
is related to MGNLSLOCK-4 Modifications to map of concurrently ... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

Sometimes the locks seem not to have been released after 3 weeks, 1 month, but 300 Seconds timeout are configured.

Probable cause could be changes/removal of collaborating user, while still in timout?
Maybe there is some missing plausibility-checking while setting up the info.



 Comments   
Comment by Wolfgang Habicht [ 08/Jul/11 ]

The problem why the timeout is not working is probably the following; however this does not explain why there are sometimes hanging users

in info.magnolia.module.softlocking.impl.DefaultSoftLockingSupport.isTimeout(Content, Long)

    protected boolean isTimeout(Content content, Long maxTimeInSeconds) {
        if(maxTimeInSeconds <= 0){
            throw new IllegalArgumentException("maxTime must be greater than 0");
        }
        final Map<String, Long> info = concurrentEditingUsers.get(content.getUUID());
        if(info == null || info.isEmpty()){
            return false;
        }
        *final String currentUserName = MgnlContext.getUser().getName();*
        *final Long lockedSince = info.get(currentUserName);*
        if(lockedSince == null){
            return false;
        }
        return System.currentTimeMillis() > (lockedSince + (maxTimeInSeconds * 1000));
    }

For me it looks like the timeout is checked for the current user only (--> info.get(currentUserName); ), not for all other users on the same node. So a "hanging" other user is not detected by the timeout

Comment by Federico Grilli [ 26/Jul/11 ]

Hello,

thanks for reporting this and sorry for my belated reply. I will look into this as soon as possible and let you know.

Best,

Federico

Comment by Federico Grilli [ 15/Aug/11 ]

Also Boris Kraft reported same problem. Here attached is a screenshot from our website author.

Comment by Federico Grilli [ 31/Aug/11 ]

Hi Wolfgang,

the isTimeout() method is actually called per user when each client (browser) polls the server for the current locking status of a given node/content. What I suspect is some unfortunate timing due to an incorrectly synchronized code reading and writing the map holding the concurrent editing users. My assumption should be also confirmed by the fact that this wrong behavior apparently shows up randomly and not on a regular basis. Thus given, the current issue seems very much related to this other one MGNLSLOCK-4. Now I'll certainly tackle the latter, and hopefully will come up with some unit tests, even though testing multithreading code isn't that easy.

Comment by Federico Grilli [ 01/Sep/11 ]

reopening the issue because, even though it should be a rare event, it may happen that a lock is not released (i.e. communication broken between browser and server). What we need is a sanity-check every time we poll the server by going through the map of concurrent editing users for a given page and removing "lingering" locks, where lingering means currentTime > lastTimeLock + timeout. Should not be an expensive operation even if performed at every polling.

Generated at Mon Feb 12 07:14:27 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.