search

Genymobile / gnirehtet

Gnirehtet provides reverse tethering for Android
Apache License 2.0
6.23k stars 571 forks source link

AdbMonitor may fail to track devices when many devices get online or offline at the same time. #396

Open mythsman opened 3 years ago

mythsman commented 3 years ago

https://github.com/Genymobile/gnirehtet/blob/f5c0710c751f7ca82ce413058abc57d9af7208c3/relay-java/src/main/java/com/genymobile/gnirehtet/AdbMonitor.java#L152

Here I think an exception should be raised to reset the current state...

rom1v commented 3 years ago

Hmmm, it should not, it's called in a loop:

https://github.com/Genymobile/gnirehtet/blob/f5c0710c751f7ca82ce413058abc57d9af7208c3/relay-java/src/main/java/com/genymobile/gnirehtet/AdbMonitor.java#L90

:confused:

Here I think an exception should be raised to reset the current state...

It should not be necessary to reset the state, if there is a bug in parsing, it should be fixed without reset.

mythsman commented 3 years ago

Hmmm, it should not, it's called in a loop:

https://github.com/Genymobile/gnirehtet/blob/f5c0710c751f7ca82ce413058abc57d9af7208c3/relay-java/src/main/java/com/genymobile/gnirehtet/AdbMonitor.java#L90

😕

Here I think an exception should be raised to reset the current state...

It should not be necessary to reset the state, if there is a bug in parsing, it should be fixed without reset.

But that exception can be caught in an outer loop: https://github.com/Genymobile/gnirehtet/blob/f5c0710c751f7ca82ce413058abc57d9af7208c3/relay-java/src/main/java/com/genymobile/gnirehtet/AdbMonitor.java#L61

and then , the state can be reset correctly.

mythsman commented 3 years ago

But I actually encouter that bug ....

mythsman commented 3 years ago

In my case ,this loop while loops forever , and hit my CPU up to 100%... https://github.com/Genymobile/gnirehtet/blob/f5c0710c751f7ca82ce413058abc57d9af7208c3/relay-java/src/main/java/com/genymobile/gnirehtet/AdbMonitor.java#L90

rom1v commented 3 years ago

But that exception can be caught in an outer loop:

The exception should occur only when a communication with the adb socket fails (so it reconnects and "restart").

In my case ,this loop while loops forever , and hit my CPU up to 100%...

OK, so there's definitely a parsing bug. To be investigated.

Does it also happen with the rust version?

rom1v commented 3 years ago

On which version of gnirehtet (which SHA-1) do you reproduce? master? dev?

mythsman commented 3 years ago

reproduce The neweset v2.5

mythsman commented 3 years ago

But that exception can be caught in an outer loop:

The exception should occur only when a communication with the adb socket fails (so it reconnects and "restart").

In my case ,this loop while loops forever , and hit my CPU up to 100%...

OK, so there's definitely a parsing bug. To be investigated.

Does it also happen with the rust version?

I don't know , I haven't tried..