RebuildReputationIndex seems to have a serious flaw?

[TSP]

I came over this by accident and not really sure what problems it'll cause, if any. But something seems very wrong to me, unless I'm missing something. 

The reason I came over it is because on this community we've never used the archive posts feature, and so I was investigating whether I could just delete the task for the \IPS\forums\Topic\ArchivedPost class. Which I at first ended up concluding I can do in theory, but then I discovered it actually changes values for rows in the reputation index anyway, even when the rows in question have already been changed for the queue task for  the IPS\forums\Topic\Post-class. 

When upgrading from 3.4 to 4.4 you kick off two background queue tasks related to rebuilding the reputation index for posts. In applications/forums/setup/upg_100018/upgrade.php:

<?php
        \IPS\Task::queue( 'core', 'RebuildReputationIndex', array( 'class' => 'IPS\forums\Topic\Post' ), 4 );
        \IPS\Task::queue( 'core', 'RebuildReputationIndex', array( 'class' => 'IPS\forums\Topic\ArchivedPost' ), 4 );

 

The problem I see here is that both queue tasks end up changing the same rows in reputation_index-table. 

This comes down to the following set of logic in public/applications/core/extensions/core/Queue/RebuildReputationIndex.php

<?php
            $update = array();
            try
            {
                $post = $classname::load( $row['type_id'] );
                // ... more code ...
            }
            catch( \OutOfRangeException $ex )
            {
                $row['item_id'] = 0;
            }

            $update['class_type_id_hash'] = md5( $classname . ':' . $row['type_id'] );

            if ( ! $row['rep_class'] )
            {
                $update['rep_class'] = $classname;
            }

            if ( \count( $update ) )
            {
                \IPS\Db::i()->update( 'core_reputation_index', $update, array( 'id=?', $row['id'] ) );
            }

 

So I created a copy of what's inside the run-function to a separate script and tried to run it manually: https://pastebin.com/ZaURWvTa

I changed between having IPS\forums\Topic\ArchivedPost and IPS\forums\Topic\Post defined as the classname. This is the output I got: 

#:~/diskusjon/upgrade/bin$ php test.php 
Running RebuildReputationIndex with class IPS\forums\Topic\Post
Found Post
class_type_id_hash (before md5): IPS\forums\Topic\Post:18143647
Want to update id 495924 in core_reputation_index:
* Update class_type_id_hash from 'f7b1358030400d369224dcaa4ce318e2' to '4833af2278742c3bcb1a5370248dc9a7'

#:~/diskusjon/upgrade/bin$ php test.php 
Running RebuildReputationIndex with class IPS\forums\Topic\ArchivedPost
Dindnt find a post, set item_id to 0
class_type_id_hash (before md5): IPS\forums\Topic\ArchivedPost:18143647
Want to update id 495924 in core_reputation_index:
* Update class_type_id_hash from 'f7b1358030400d369224dcaa4ce318e2' to 'f7b1358030400d369224dcaa4ce318e2'

 

This means a row in core_reputation_index could have the wrong hash, dependent upon which of the tasks hit the row in the table first. Both instances (Post and ArchivedPost) of this task runs on the same rows. 

As an added note: when it encounters the \OutOfRangeException trying to find the post, it sets $row['item_id'] to 0, but it doesn't actually use that value anywhere further on. 

 

[bfarber]

I've logged an internal bug report for this to be investigated.

[TSP]

12 hours ago, bfarber said:

I've logged an internal bug report for this to be investigated.

Thanks. For now I've decided to end the RebuildReputationIndex-task for ArchivedPost and instead restart the task for Post-class.