Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't compare two blank moment() for potential race condition #14974

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

harumhl
Copy link

@harumhl harumhl commented Oct 26, 2023

#10225

Changes

When appearanceSets[setKey].availableUntil is undefined, then we were performing moment().isBefore(moment()) and the race condition occasionally returns true about 0.05-0.5% of the time (from my experience).

How to test:

  1. Go to website/client/src/mixins/avatarEditUtilities.js
  2. Change hide = this.hideSet(set); to hide = this.hideSet(set, key);
  3. Change hideSet (setKey) to hideSet (setKey, key)
  4. Add below code after if (appearanceSets[setKey].availableFrom) block
      if (key === 'bear') {
        for (let i = 0; i < 10000; i += 1) {
          const tempHide = moment(appearanceSets[setKey].availableUntil).isBefore(moment());
          if (tempHide) {
            console.log(tempHide);
          }
        }
      }
  1. Run the local instance of Habitica
  2. Click User icon > Click 'Edit Avatar' > Click 'Skin'
  3. You should see a few true cases on Console
  4. Now change the above const tempHide = ... with below block (which has the identical logic as this PR)
          const tempHide = appearanceSets[setKey].availableUntil
            ? moment(appearanceSets[setKey].availableUntil).isBefore(moment())
            : false;
  1. Repeat steps 6
  2. Now you don't see any true printed out on Console
  • The key above doesn't have to be 'bear'. I just wanted a key that will return hide to be false by default. You can try any key for Skin that should return hide = false (e.g. 'rainbow')

UUID: 662332a6-a7e7-4b70-858e-255f3e6076a8

@harumhl harumhl changed the title if appearanceSets[setKey].availableUntil doesn't exist, don't compare… Don't compare two blank moment() for potential race condition Oct 26, 2023
@harumhl harumhl marked this pull request as ready for review October 26, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant