Skip to content
This repository has been archived by the owner on Aug 25, 2022. It is now read-only.

readBytes fix-read-endless-loop #196

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

yufewell
Copy link

fix readBytes endless loop

@peter279k
Copy link
Contributor

@yufewell, could you have any test to prove this change?

@yufewell
Copy link
Author

@yufewell, could you have any test to prove this change?

yes, cut off wi-fi then readBytes will be endless loop,
i find that the stream's timed_out become true,
hope it helpful.

Copy link
Contributor

@Taluu Taluu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we would break out of a loop to stop reading but okay. The reading mechanism is flawed anyways...

src/Engine/AbstractSocketIO.php Outdated Show resolved Hide resolved
src/Engine/AbstractSocketIO.php Outdated Show resolved Hide resolved
@yufewell
Copy link
Author

Not sure why we would break out of a loop to stop reading but okay. The reading mechanism is flawed anyways...
this is only happen while network is nearly turned off.
when network is terrible, fread alwarys return empty string, so the stream will become timed out, then the loop will last forever.
in my case, i need break to reconnect to server...

yufewell and others added 2 commits November 19, 2019 12:05
Co-Authored-By: Baptiste Clavié <clavie.b@gmail.com>
Co-Authored-By: Baptiste Clavié <clavie.b@gmail.com>
@yufewell
Copy link
Author

Not sure why we would break out of a loop to stop reading but okay. The reading mechanism is flawed anyways...

also can use keepAlive function to reconnect...

Copy link
Contributor

@Taluu Taluu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, not sure if I'm truly convinced this patch is needed.

But I can be wrong...

src/Engine/AbstractSocketIO.php Outdated Show resolved Hide resolved
src/Engine/SocketIO/Version1X.php Outdated Show resolved Hide resolved
Comment on lines +118 to +120
if ($i > 2) {
throw new ServerConnectionFailureException('fread times error');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems pretty arbitrary that after 2 iterations, we would make it die.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i did not find correct way to solve endless loop problem.
when fread always return empty string, cpu will went high and loop will not break off.
i tried some ways, timed_out, is_resource... but did not work...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What could be done would be to temper with pcntl signals to stop the loop then. WDYT ?

@guillaumepotier
Copy link
Member

To be honest, not sure if I'm truly convinced this patch is needed.
But I can be wrong...

You, wrong ? Nah !!!! :trollface:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants