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

Fix NullPointerException when server ticking #581

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

zyxkad
Copy link
Collaborator

@zyxkad zyxkad commented Apr 14, 2024

PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION

  • Please check if the PR fulfills these requirements
  • The commit message are well described
  • Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. This is not mandatory
  • All changes have fully been tested
  • What kind of change does this PR introduce? (Bug fix, feature, ...)

Some mod may call ServerWorker.tick with null value, this PR ensure null value never go to the queue, and if it did somehow, the null will never be executed

  • What is the current behavior? (You can also link to an open issue here)

  • What is the new behavior (if this is a feature change)?

  • Does this PR introduce a breaking change? (What changes might users need to make in their scripts due to this PR?)
    No

  • Other information:

@SquidDev
Copy link
Contributor

What bug is this trying to fix? I don't see any usages of ServerWorker.add that enqueue null.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Apr 14, 2024

@SquidDev
Copy link
Contributor

That's not very helpful!. I suspect the issue here is actually due to the non-thread-safe usage of ArrayQueue. This should either use a concurrent queue, or obtain a lock on the queue.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Apr 14, 2024

Ok then:
Error Log

@zyxkad
Copy link
Collaborator Author

zyxkad commented Apr 14, 2024

I suspect the issue here is actually due to the non-thread-safe usage of ArrayQueue. This should either use a concurrent queue, or obtain a lock on the queue.

You are right, they are running concurrently

zyxkad added a commit to zyxkad/AdvancedPeripherals that referenced this pull request Apr 14, 2024
@19PHOBOSS98
Copy link

merge please : )

@zyxkad
Copy link
Collaborator Author

zyxkad commented Apr 22, 2024

@19PHOBOSS98 can you build your own version based on this PR and test if that error never happen again?

Cause it's not really easy for me to reproduce

@SirEndii SirEndii merged commit 0ef4a8c into IntelligenceModding:dev/1.20.1 Apr 22, 2024
2 checks passed
@19PHOBOSS98
Copy link

@19PHOBOSS98 can you build your own version based on this PR and test if that error never happen again?

Cause it's not really easy for me to reproduce

sure thing! I'll be back with what I find

@19PHOBOSS98
Copy link

it doesn't crash anymore when I tried setting the threads to 4 with all of my drones flying with VS2. Thanks again! Hope this releases soon

@zyxkad zyxkad deleted the patch-2 branch April 26, 2024 23:33
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.

4 participants