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

Avoid using getmypid #15364

Merged
merged 4 commits into from Jan 9, 2020
Merged

Avoid using getmypid #15364

merged 4 commits into from Jan 9, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jan 9, 2020

@diosmosis was getting this error a few times:

image

I wonder if we could simply generate a random number instead of using getmypid? I noticed the class is always used through DI so in the end the instance should be only created once and therefore the pid should always be the same even though the property is not static. Could also make it static though if that helps and only set it once?

Not too important for 3.13.1 but be good. We can move it to 4.0 though and I patch later Matomo for WordPress after the 3.13.1 release

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jan 9, 2020
@tsteur tsteur added this to the 3.13.1 milestone Jan 9, 2020
@diosmosis
Copy link
Member

It might need to be cached and the same one used for the entire run (not too sure, just thinking)

@tsteur
Copy link
Member Author

tsteur commented Jan 9, 2020

@diosmosis was adding the cache but then noticed this was making things worse as cache can be reset any time and then new PID was generated. The original simple logic worked actually fine after looking into it a bit more but figured better to create a method in Common class for this purpose since we had similar logic somewhere else. First had it Common::getMyPid but this could lead to wrong assumption when not reading the text so named it getProcessId.

@diosmosis diosmosis merged commit be5a28b into 3.x-dev Jan 9, 2020
@diosmosis diosmosis deleted the getmypidavoid branch January 9, 2020 21:11
jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
* Avoid using getmypid

* use cache for pid

* revert previous change and not use cache

* Added method for getmypid
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
* Avoid using getmypid

* use cache for pid

* revert previous change and not use cache

* Added method for getmypid
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants