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

Some events are not emitted in some Android devices #54

Closed
mateo-gallardo opened this issue Jun 15, 2020 · 17 comments
Closed

Some events are not emitted in some Android devices #54

mateo-gallardo opened this issue Jun 15, 2020 · 17 comments
Labels
android bug Something isn't working released

Comments

@mateo-gallardo
Copy link

Description

On some Android devices some events emitted via the sendEvent method in TcpSocketModule are not received in the JS side.
For some reason it seems to be fixed with a Thread.sleep(100) right before any event is emitted in the TcpReceiverTask. Right now I have it before the instantiating the BufferedInputStream and it doesn't fail, but I don't understand why.

Thread.sleep(100);
BufferedInputStream in = new BufferedInputStream(socket.getInputStream());
while (!isCancelled() && !socket.isClosed()) {

Do you have any idea why this could be happening?
I'm guessing it's giving time for another task in another thread to be ready for these events, but I don't know what it is.

Steps to reproduce

Steps to reproduce the behavior:
It's hard to reproduce because it doesn't happen in every device. I've got a Pixel 3 where it works perfectly and a HUAWEI MediaPad 5 where it fails quite often.

I create a connection, send the data and close it immediately.
The Pixel 3 failed 0 out of 100 events emitted.
The HUAWEI tablet failed 51 out of 100 events emitted.
The HUAWEI tablet with the Thread.sleep(100) failed 0 out of 100 events emitted.

Relevant information

| OS | Android |
| react-native | 0.62.2 |
| react-native-tcp-socket | 3.7.1 |

@mateo-gallardo mateo-gallardo added the bug Something isn't working label Jun 15, 2020
@Rapsssito
Copy link
Owner

@mateo-gallardo, thanks for your detailed explanation! Could you provide the code you used to test it?

@mateo-gallardo
Copy link
Author

mateo-gallardo commented Jun 16, 2020

The server would concatenate all the JSON data received during the connection and parse it on close, that's how I know it failed, at no point the data event is fired and the parsing fails with Unexpected EOF.

let errorCount = 0;

TcpSocket.createServer(socket => {
  let json = '';

  socket.on('data', data => {
    console.log(`Received data ${data.toString()}`);
    json += data.toString();
  });

  socket.on('error', error => {
    console.log(`An error ocurred with client socket ${JSON.stringify(error)}`);
  });

  socket.on('close', () => {
    try {
      // Parsing on close since onData could be called several times with parts of the JSON for big JSONs
      const parsed = JSON.parse(json);
      console.log('Parsed successfully');
    } catch (error) {
      errorCount++;
      console.log(`Error parsing JSON ${error.message}. Error count: ${errorCount}`);
    }

    console.log('Closed connection');
  });
}).listen({
  port: 12345,
  host: '0.0.0.0'
}, serverInfo => {
  console.log(`TCP Socket open on ${serverInfo.address}:${serverInfo.port}`);
});

this.server.on('error', error => {
  console.log(`An error ocurred with the server ${JSON.stringify(error)}`);
});

this.server.on('close', () => {
  console.log('Server closed connection');
});

The client I'm using is actually a native iOS app, but any connection will do. Just a 0..100 for loop that sends the same JSON.

@Rapsssito
Copy link
Owner

@mateo-gallardo, thanks for the code! As I understand it, the issue is that the 'close' event is being called before the 'data' event, even though the socket sends the data before it closes the connection. Is that right?

@mateo-gallardo
Copy link
Author

That was my suspicion at first, but logging the native Android code revealed that the events where being fired, just not received on the JS side.
That Thread.sleep 'fixes' it for some reason, even with a 1ms sleep I got 1 error of out 300 tries.

I did quite a bit googling, but I couldn't find anything related to RN's events and background threads.

@Rapsssito
Copy link
Owner

Rapsssito commented Jun 16, 2020

@mateo-gallardo, I think it is related to RCTDeviceEventEmitter not being thread safe. I will get in touch as soon as I find something. Is this also happening on iOS?

@mateo-gallardo
Copy link
Author

Great thanks! Let me know if you have any theories/code that I can test out.
iOS works just fine 👍

@mateo-gallardo
Copy link
Author

I tried running the sendEvent method in the main thread posting Runnables in Handler (new Handler(Looper.getMainLooper())) like they do in this PR but no luck.

I haven't tried queuing them up and sending them FIFO but I assume that running them in the same thread would guarantee that.

@Rapsssito
Copy link
Owner

@mateo-gallardo, could you try changing sendEvent() from private void to private synchronized void?

@mateo-gallardo
Copy link
Author

Just tried it, sadly it didn't work.

@Rapsssito
Copy link
Owner

@mateo-gallardo, I cannot reproduce the issue by myself. Could you provide me with a minimal reproducible code that I could test against?

@mateo-gallardo
Copy link
Author

The main factor with this issue is hardware, as I said in the Pixel 3 works fine but in the Huawei tablet it doesn't.
I'll make a minimal app with the TCP server and a NodeJS client that sends a lot of JSONs and get back to you.

@Rapsssito
Copy link
Owner

Rapsssito commented Jun 17, 2020

@mateo-gallardo thanks so much! I also have a Pixel 3, but I know a couple friends with different Android phones. With your minimal code I would be able to get to the solution much faster.

@mateo-gallardo
Copy link
Author

Ok, here's the app that will display how many errors parsing the JSON it gets.
https://github.com/mateo-gallardo/tcp-socket-error-test-app

App image that shows an error count of 36

And here's the "client" that'll connect to the app and send a JSON every 1 second until you quit the process. Make sure you change the IP in the index.js file before running it.
https://github.com/mateo-gallardo/tcp-socket-error-test-client

Screenshot of the nodejs console

It fails quite often in the Huawei tablet, but never fails in my Pixel 3.

@Rapsssito
Copy link
Owner

@mateo-gallardo, using your code changing the JSON timeout to 1 millisecond I am able to reproduce the Unexpected EOF exception in a Pixel 3. I am investigating.

@Rapsssito
Copy link
Owner

@mateo-gallardo, I think I found something. Could you try to swap the lines 32 and 33 in TcpSocketServer.java? It would look like this:

Socket socket = serverSocket.accept();
int clientId = getClientId();
TcpSocketClient socketClient = new TcpSocketClient(mReceiverListener, clientId, socket);
socketClients.put(clientId, socketClient);
// socketClient.startListening(); // Comment this line
mReceiverListener.onConnection(getId(), clientId, new InetSocketAddress(socket.getInetAddress(), socket.getPort()));
socketClient.startListening(); // Add this line

@mateo-gallardo
Copy link
Author

YES! That fixed it, thanks!
Works perfectly in the example app and the app I'm working on. Great job!

github-actions bot pushed a commit that referenced this issue Jun 18, 2020
# [4.1.0](v4.0.0...v4.1.0) (2020-06-18)

### Bug Fixes

* **Android:** Fix server socket events not being delivered ([50e9b79](50e9b79)), closes [#54](#54)

### Features

* Add setTimeout() method ([#56](#56)) ([e642e1a](e642e1a))
@github-actions
Copy link

🎉 This issue has been resolved in version 4.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android bug Something isn't working released
Projects
None yet
Development

No branches or pull requests

2 participants