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

Message to Groupchat with Roaming mode renders phone useless #1441

Closed
PulsarFX opened this issue Apr 29, 2014 · 13 comments
Closed

Message to Groupchat with Roaming mode renders phone useless #1441

PulsarFX opened this issue Apr 29, 2014 · 13 comments

Comments

@PulsarFX
Copy link

We were on the road when I wrote a message to a groupchat. When I was ready for sending the message, my phone has gone to roaming mode and disabled data channel.
TS did try to fall back to SMS which failes for group chats. Then a notification is shown like "SMS could not be sent, will be resent later". There seems to be an aggressive loop for this, as this notification didn't go away from my screen until my phone went out of roaming mode again. It did stay there whichever app I was starting...

@liliakai
Copy link
Contributor

liliakai commented May 2, 2014

I was able to repro this by trying to send a group chat in Airplane Mode. It didn't kill my whole phone, and resolved itself pretty quickly once I turned off Airplane Mode, but the experience could definitely be smoother.

@mcginty
Copy link
Contributor

mcginty commented May 2, 2014

Definitely counts as a bug, that's pretty confusing. Thanks for reporting!

@jocelynthode
Copy link

Being in Airplane mode didn't help me reproduce the bug, for the bug to appear, you have to have no data connectivity, no wifi, but still being able to call and send sms. I'll look into this to see if I can fix it.

@mcginty
Copy link
Contributor

mcginty commented May 6, 2014

@jocelynthode I believe #1444 includes a decent fix for this already

@jocelynthode
Copy link

@mcginty ah yeah sorry didn't see you referenced this issue, nevermind then.

@mcginty
Copy link
Contributor

mcginty commented May 6, 2014

@jocelynthode I take it back, I just tested and it doesn't - separate bug.

@jocelynthode
Copy link

@mcginty ok I'll then still look into it and see if I can do it or not.

@mcginty
Copy link
Contributor

mcginty commented May 6, 2014

thanks!

@jocelynthode
Copy link

I may have found something in MMSSender, but I'm not sure if this is the case as I can't test it at the moment.

if (systemStateListener.isConnected()) scheduleQuickRetryAlarm();
else                                   systemStateListener.registerForConnectivityChange();

I think that NetworkInfo returns null if you're in airplane mode (which SystemStateListener checks for) but still returns true even when there are no internet access (only wifi and data channel disabled). Which might trigger the scheduleQuickRetry alarm in loop.

Is this possible ?

I'll do more tests once I'm home .

@mcginty mcginty removed the easy label May 7, 2014
@jocelynthode
Copy link

I found this exception in the log. Any help ? (I redacted the phone number)

W/PushServiceSocket(22477): Push service URL: https://textsecure-service.whispersystems.org
W/PushServiceSocket(22477): Opening URL: https://textsecure-service.whispersystems.org/v1/messages/+****************
W/SystemStateListener(22477): Registering for any connectivity changes...
W/SystemStateListener(22477): java.lang.IllegalArgumentException: Receiver not registered: org.thoughtcrime.securesms.service.SystemStateListener$ConnectivityListener@40e72048
W/SystemStateListener(22477):   at android.app.LoadedApk.forgetReceiverDispatcher(LoadedApk.java:657)
W/SystemStateListener(22477):   at android.app.ContextImpl.unregisterReceiver(ContextImpl.java:1391)
W/SystemStateListener(22477):   at android.content.ContextWrapper.unregisterReceiver(ContextWrapper.java:445)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.service.SystemStateListener.unregisterForConnectivityChange(SystemStateListener.java:46)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.service.SystemStateListener.registerForConnectivityChange(SystemStateListener.java:36)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.service.MmsSender.handleSendMms(MmsSender.java:109)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.service.MmsSender.process(MmsSender.java:60)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.service.SendReceiveService$SendReceiveWorkItem.run(SendReceiveService.java:275)
W/SystemStateListener(22477):   at org.thoughtcrime.securesms.util.WorkerThread.run(WorkerThread.java:46)

@Hellmy
Copy link

Hellmy commented Jun 17, 2014

Think I tracked it down but first wrote a mail about some solutions.

@Hellmy
Copy link

Hellmy commented Nov 24, 2014

My thoughts from the mailing list without any result =/
or has already something happend?

I think the problem is in SystemStateListener.java.
There are two listeners registered, the overall listener for a change of
the service_state and one listener for connectivity_change...
Both are triggering quite often (first point) and the real problem is
the TelephonyListener because there is no check of a data connection
so we loop all the time with the mms.

I want to ask and discuss how to fix this (also perhaps change there a
bit more?)

  1. Fast and "dirty": Just remove the sendMmsOutbox from TelephonyListener
  2. Think about the intented changes of the connectivity and register
    just that events (also change the calls). Perhaps a bit more complicated
    because it's no more a one for all event listening (so you have to
    unregister carefully.
  3. Check also during the TelephonyListener callback about the data
    connection and only trigger when data is active

I could make the implementation 1 and 3 in a few minutes but perhaps
there are some arguments about two listeners.

@rhodey
Copy link
Contributor

rhodey commented Apr 10, 2015

no longer able to reproduce on 2.10.2, closing.

@rhodey rhodey closed this as completed Apr 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants