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: avoid prime worker message spam. #764

Closed

Conversation

dvogel
Copy link

@dvogel dvogel commented Feb 23, 2020

The prime worker can receive messages posted by browser extensions (e.g. react-devtools-bridge). Since the message lacked the necessary inputs, the prime algorithm would start but never terminate. This drops useless messages on the floor.

The prime worker can receive messages posted by browser extensions (e.g.
react-devtools-bridge). Since the message lacked the necessary inputs,
the prime algorithm would start but never terminate.
@davidlehn davidlehn added this to the v1.x milestone Jan 5, 2022
@davidlehn
Copy link
Member

I'm not sure what to do with this.

On one hand, I can see how getting arbitrary messages could cause problems. And I can see how this is a simple change that would fix it (other than in the edge case where the arbitrary message had the same fields).

On the other hand, I can't see why a browser extension would send arbitrary messages to a worker and not expect havoc to ensue. What is being sent, and why?

Is there somewhere that suggests best practices for this?

@dlongley
Copy link
Member

dlongley commented Jan 7, 2022

I think it's fine to ensure that we're only acting on messages that are intended to be received by the worker. However, we should add a more specific message type property (with a value that is an identifier that specifies the message is a forge prime worker message) that we can check rather than relying on looking for e.data.hex and e.data.workLoad.

@davidlehn
Copy link
Member

Tried another approach in #941.

@dlongley
Copy link
Member

Closing in favor of #941, which should be merged.

@dlongley dlongley closed this Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants