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

fix tcp read unstable stuck #25

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

Conversation

cs8425
Copy link

@cs8425 cs8425 commented Feb 10, 2017

fix tcp read sometimes stuck on android platform.
cause by sometimes sendReceiveEvent() are un-ordered.

sendReceiveEvent(new PluginResult(Status.OK, info));
sendReceiveEvent(new PluginResult(Status.OK, recvBytes));

@mcelrath
Copy link

mcelrath commented Mar 2, 2017

Can you explain your logic here? You're calling readyToRead at least twice for each packet. Once should be enough, if it needs to be called at all.

I find that not calling readyToRead at all seems to solve #26.

@cs8425
Copy link
Author

cs8425 commented Mar 4, 2017

fix the same issue on android as @mcelrath at #27 .

exec(null, readyToRead, 'ChromeSocketsTcp', 'readyToRead', [socketId])
Call js function readyToRead only when exec failed, not twice for each packet.
https://cordova.apache.org/docs/en/latest/guide/hybrid/plugins/#the-javascript-interface
Just make sure native readyToRead will be called once.

@mcelrath
Copy link

mcelrath commented Mar 5, 2017 via email

@cs8425
Copy link
Author

cs8425 commented Mar 5, 2017

@mcelrath
If not call native readyToRead,
nio will never go to read any new data from socket (SelectionKey.OP_READ has been removed),
but you still can sent data (can write, can't read).
You can look some java nio program document about channels and selector.
http://tutorials.jenkov.com/java-nio/selectors.html
And try not to call native readyToRead on a bidirectional, keep sending tcp socket, then you will know what I said.

ps. make sure the code has been updated, check by unzip the output apk file.

@mcelrath
Copy link

mcelrath commented Mar 5, 2017 via email

@cs8425
Copy link
Author

cs8425 commented Mar 7, 2017

@mcelrath
your patch can't work...
as I said, never go to read any new data!
only signature and /assets/www/plugins/cordova-plugin-chrome-apps-sockets-tcp/sockets.tcp.js are different.
you can unzip/disassembling apk file to check it.

app source: https://github.com/cs8425/tcp-test-app
pre-build apk
mine: google drive
your: google drive

@mcelrath
Copy link

mcelrath commented Mar 7, 2017 via email

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.

2 participants