1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Future Fix Updating user conversation counters is deadlock prone.

Discussion in 'Future-Fix Bug Reports' started by Xon, Aug 24, 2015.

  1. Xon

    Xon Well-Known Member

    Every so often, I'm getting exception originating from inside XenForo_Model_Conversation::rebuildUnreadConversationCountForUser().

    Looking at the code-flow, it is a design issue where
    rebuildUnreadConversationCountForUser() is accessing other user's conversations inside a transaction which is modifying their own conversations.

    This leads to a potential interleaving of updates & selects on the same rows which can cause a deadlock.

    Moving the rebuildUnreadConversationCountForUser() call into XenForo_DataWriter_ConversationMaster::_postSaveAfterTransaction effectively fixes this, as it break apart interleaving update/select/update statements.

    This is a fairly minor thing, but it crops up every so often.
  2. Xon

    Xon Well-Known Member

    Note; XenForo_DataWriter_ConversationMessage also chains to XenForo_DataWriter_ConversationMaster, which can pull the _postSaveAfterTransaction function into a transaction thus defeating trying to move the call to rebuildUnreadConversationCountForUser out of a transaction.
  3. Mike

    Mike XenForo Developer Staff Member

    I think I may have to flag this for a future version, as the sort of changes needed don't really fit with a minor bug fix release IMO. I could handle deferring the update a bit, though as you've noted, there are still ways to trigger this inside a transaction. We don't have a straightforward framework for handling deferring activities until later in the request (such as on transaction commit or at the end of the request). These actually become rather straightforward if you can actually use closures...

    That said, this might actually be moot in XF 2 anyway due to some changed approaches... ;)
    Xon likes this.
  4. Xon

    Xon Well-Known Member

    I thought it was running into some design limits, and didn't look like it could be cleanly fixed.

    In that case, I'll leave my workaround in my Conversation Improvements add-on :)

    Well XF1.5's minimum supported version is recent enough that php does have closures. But that would also require building a, relatively trivial, framework for it.

    Coming Real Soon Now (tm)?
  5. Mike

    Mike XenForo Developer Staff Member

    We still support PHP 5.2. :( (The comment regarding < 5.4 when upgrading is deprecation, but it still functions on older versions.)
    Xon likes this.

Share This Page