Z-Push
Z-Push copied to clipboard
Z-Hub
Duplicate-Pings-With-Same-Timestamp
Released under the GNU Affero General Public License (AGPL), version 3 and Trademark Additional Terms.
What does this implement/fix? Explain your changes.
This code detects when two or more Ping Requests from the same device/user have the same start timestamp and tells the ones with the smaller PID to self-terminate
Does this close any currently open issues?
Fixes issue #55
Any relevant logs, error output, etc?
Initial check-in includes DEBUG logs to demonstrate the functionality. These should be removed once the fix has been accepted.
Line 1196: 16/02/2024 16:08:17 [1496456] [DEBUG] MultiPINGS: Other process [31200] starttime same as mine : Other process ID is "smaller" than mine - return FALSE
Line 1197: 16/02/2024 16:08:17 [1496456] [DEBUG] MultiPINGS: This is my PID - Ignore me - return FALSE
Line 1503: 16/02/2024 16:09:17 [31200] [DEBUG] MultiPINGS: This is my PID - Ignore me - return FALSE
Line 1504: 16/02/2024 16:09:17 [31200] [DEBUG] MultiPINGS: Other process [1496456] starttime same as mine : Other process ID is "bigger" than mine - return TRUE
Where has this been tested?
Server (please complete the following information):
- OS: Rocky Linux 9
- PHP Version: 8.1
- Backend for: zimbra
- and Version: 2.7.1
Smartphone (please complete the following information):
- Device: Samsung S10
- OS: Android 12
- Mail App GMail
- Version Android-Mail/2024.01.28...
Thanks @liverpoolfc-fan
Always appreciate your PRs
I won't have a chance to test this for the next couple of weeks. But will do it as soon as I can.
Cheers, Mat
Grommunio-sync addressed this issue by using microtime() instead of time() to track the start time. Due to the derived code bases it's probably easiest to change this in the inter process data class. https://github.com/Z-Hub/Z-Push/blob/develop/src/lib/core/interprocessdata.php#L116
I've tested it out and it looks good. Have you tested out @grammmichi's comment?
Happy to merge this either way if you want to remove the debug log lines.
Cheers, Mat
Grommunio-sync addressed this issue by using microtime() instead of time() to track the start time. Due to the derived code bases it's probably easiest to change this in the inter process data class. https://github.com/Z-Hub/Z-Push/blob/develop/src/lib/core/interprocessdata.php#L116
I tried setting start to microtime and it breaks z-push immediately in a few places. I am sure all are fixable but there could be more places that it will affect.
Fatal error: /usr/share/Z-Push-2.7.1/src/lib/utils/utils.php:721 - Uncaught TypeError: strftime(): Argument #2 ($timestamp) must be of type ?int, string given in /usr/share/Z-Push-2.7.1/src/lib/utils/utils.php:721 /usr/share/Z-Push-2.7.1/src/z-push-top.php:252 A non-numeric value encountered (2) /usr/share/Z-Push-2.7.1/src/z-push-top.php:286 A non-numeric value encountered (2) /usr/share/Z-Push-2.7.1/src/lib/core/loopdetection.php:199 A non-numeric value encountered (2)
For now I think we should go with my suggested fix
I've tested it out and it looks good. Have you tested out @grammmichi's comment?
Happy to merge this either way if you want to remove the debug log lines.
Cheers, Mat
I have removed the DEBUG lines and added a Comment instead.
Do I need to do anything else at this time, or is the same Pull request still good for it?
Vincent