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

sendAndReceive works endlessly (timeout is ignored) #73

Closed
galpeaceman opened this issue Jan 25, 2021 · 8 comments
Closed

sendAndReceive works endlessly (timeout is ignored) #73

galpeaceman opened this issue Jan 25, 2021 · 8 comments
Labels

Comments

@galpeaceman
Copy link

galpeaceman commented Jan 25, 2021

I have a working app using this module.
It works perfectly for days, sampling thousands of samples from a single device.
Then, one day, the CPU goes up to 100% and works endlessly until I kill it.

I think I understand the source of the problem.
See ModbusTcpClient\Network\StreamHandler's receiveFrom method (I copied here the relevant part of the code):

        if ($timeout === null) {
            $timeout = 0.3;
        }
        ...
        $lastAccess = microtime(true);
        ...
        while ($responsesToWait > 0) {
            ...
            $dataReceived = false;
            foreach ($read as $stream) {
                ...
                if ($index !== null) {
                    ...
                    $data = fread($stream, 256); // read max 256 bytes
                    if (!empty($data)) {
                        ...
                        $dataReceived = true;
                    }
                }
            }
            if (!$dataReceived) {
                $timeSpentWaiting = microtime(true) - $lastAccess;
                if ($timeSpentWaiting >= $timeout) {
                    throw new IOException('Read total timeout expired');
                }
            }
            $lastAccess = microtime(true);

So, what if fread fails very quickly (due to a connection timeout) and $dataReceived is false;
$timeSpentWaiting would be a very small number, possibly less than $timeout.
No matter, after a few tries $timeout would pass, right? No. It will never pass. Why?
Because $lastAccess is reset again to microtime(true) and the whole thing repeats itself.

I think the correct code should be:

            if (!$dataReceived) {
                $timeSpentWaiting = microtime(true) - $lastAccess;
                if ($timeSpentWaiting >= $timeout) {
                    throw new IOException('Read total timeout expired');
                }
            } else {
                $lastAccess = microtime(true);
            }

What do you say?

@aldas
Copy link
Owner

aldas commented Jan 25, 2021

first idea would be - maybe machine clock is being synchronized "back in time". I have to investigate how php acts when clock 'drifts'. Do you have NTP or some automatic time synchronization setup for your server?

@galpeaceman
Copy link
Author

Though 'drifting' is possible, it is not required to explain the phenomena; since the $lastAccess resets itself at every iteration, it is very possible that $timeout did not pass yet

@aldas
Copy link
Owner

aldas commented Jan 25, 2021

You are correct. It is definitely problem when stream_select returns early and fread does not produce data. Probably something happens with connection. I guess fread would return boolean false in that case for you.

Can you modify your code (vendor folder) in production? And does that problem occur reliably?

You could try

                    $data = fread($stream, 256); // read max 256 bytes
                    if ($data === false) {
                        throw new IOException('fread error during receiveFrom');
                    }
                    if (!empty($data)) {

(in addition that if (!$dataReceived) {} else {} change)

meanwhile I fix this in master and release new version

aldas added a commit that referenced this issue Jan 25, 2021
…s early but `fread` does not produce data and $lastAccess getting reset and moving timeout always into future (#73)
aldas added a commit that referenced this issue Jan 25, 2021
…s early but `fread` does not produce data and $lastAccess getting reset and moving timeout always into future (#73)
@aldas
Copy link
Owner

aldas commented Jan 25, 2021

It is in master branch now. I'll do proper release in couple of days

@galpeaceman
Copy link
Author

I have made the change and lets wait and see.
Unfortunately, this does not reproduce often; last time it happened was 6 weeks ago.
Let's assume this will solve the problem, and if it persists, I will reopen the issue.
Thanks!

@aldas
Copy link
Owner

aldas commented Jan 25, 2021

@galpeaceman are you really running same PHP process for weeks? I use this library in industrial environment (4+ year now) and out of fears of memory leaks and stuff I built my modbus poller to run X amount of time and then automatically stop/restart + guards not to run multiple instance etc. I think I'm quite paranoid :)

@galpeaceman
Copy link
Author

Well, I did not mean it is the same PHP process.. it currently resets after 5 reads, just like you described.
But we do intend to increase the read count per process greatly, once we solve the crash, we will do that.
I just meant that it makes a read every minute for a few weeks without errors, and then one morning it just freezes.

@aldas
Copy link
Owner

aldas commented Jan 29, 2021

New version is published - I'll close this issue for now. And @galpeaceman thanks for pointing it out!

@aldas aldas closed this as completed Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants