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.

Sign up for GitHub

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

Jump to bottom

Fix potential race in start/stop #16

Open
gavv opened this issue Dec 3, 2020 · 1 comment
Open

Fix potential race in start/stop #16

gavv opened this issue Dec 3, 2020 · 1 comment
Assignees
@gavv
Labels
defect Something isn't working

Comments

@gavv
Copy link
Member

gavv commented Dec 3, 2020 •
edited
Loading

There is a potential race in startStopReceiver and startStopSender.

    fun startStopReceiver(@Suppress("UNUSED_PARAMETER") view: View) {
        if (senderReceiverService.isReceiverAlive()) {
            senderReceiverService.stopReceiver()
        } else {
            senderReceiverService.startReceiver()
        }
    }

Assume that the user presses STOP, startStopReceiver() is invoked, and it calls stopReceiver(). Then receiver thread receives interrupt exception and invokes senderChanged(false). UI thread handles it and renames button from STOP to START. Then user presses START button and startStopReceiver() is invoked again.

The code above expects that in this situation isReceiverAlive() will return false, so that we will call startReceiver().

However, it's possible that the receiver thread already called senderChanged(), but didn't finish yet, and isReceiverAlive() will report true. In result, the user presses START, but we call stopReceiver() and don't try to start anything.

This race is present for both receiver and sender.
@gavv gavv added defect Something isn't working good first issue Good for newcomers help wanted Contributions are welcome labels Dec 3, 2020
@gavv gavv mentioned this issue Dec 3, 2020
Merged
@gavv gavv changed the title Fix potential race Fix potential race in start/stop Feb 5, 2023
@gavv
Copy link
Member Author

gavv commented Oct 5, 2024

Will be fixed in #98

@gavv gavv self-assigned this Oct 5, 2024
@gavv gavv removed good first issue Good for newcomers help wanted Contributions are welcome labels Oct 5, 2024
gavv added a commit to Izchomatik/roc-droid that referenced this issue Oct 5, 2024
@gavv
Android backend overhaul and fixes (roc-streaming#98)
4335b7e 
Also fixes: roc-streaming#1, roc-streaming#16, roc-streaming#17, roc-streaming#66

- Initiate request for notification and microphone permissions
  and media projection on the dart side of AndroidBackend.

- Implement those requests in MainActivity.kt and rework
  AndroidConnector to redirect those requests to activity.

- Automatically stop media projection when both sender and receiver
  are stopped. Prevent service from stopping projection while we
  are starting sender and receiver.

- Implement synchronization and fix various races.

- Use foreground service instead of bound service, to keep it
  running when app closes.

- Forbid swiping away notification on lock screen.
  (We can forbid it only for lock screen).

- Stop sender and receiver when notification is swiped away.

- Add AndroidListener, to pass events from kotlin to dart.
  Implement in in AndroidBackend on dart side.

- Add AndroidSenderSettings, AndroidReceiverSettings, and pass them
  from model to kotlin.

- Hard-code ports in model instead of kotlin.

- Implement Backend.getLocalAddresses().

- Improve comments.

- Refactor android service code.

- Remove unused values from strings.xml.
gavv added a commit to Izchomatik/roc-droid that referenced this issue Oct 5, 2024
@gavv
Android backend overhaul and fixes (roc-streaming#98)
a95456d 
Also fixes: roc-streaming#1, roc-streaming#16, roc-streaming#17, roc-streaming#66

- Initiate request for notification and microphone permissions
  and media projection on the dart side of AndroidBackend.

- Implement those requests in MainActivity.kt and rework
  AndroidConnector to redirect those requests to activity.

- Automatically stop media projection when both sender and receiver
  are stopped. Prevent service from stopping projection while we
  are starting sender and receiver.

- Implement synchronization and fix various races.

- Use foreground service instead of bound service, to keep it
  running when app closes.

- Forbid swiping away notification on lock screen.
  (We can forbid it only for lock screen).

- Stop sender and receiver when notification is swiped away.

- Add AndroidListener, to pass events from kotlin to dart.
  Implement in in AndroidBackend on dart side.

- Add AndroidSenderSettings, AndroidReceiverSettings, and pass them
  from model to kotlin.

- Hard-code ports in model instead of kotlin.

- Implement Backend.getLocalAddresses().

- Improve comments.

- Refactor android service code.

- Remove unused values from strings.xml.
gavv added a commit that referenced this issue Oct 5, 2024
@gavv
Android backend overhaul and fixes (#98)
64ac7a4 
Also fixes: #1, #16, #17, #66

- Initiate request for notification and microphone permissions
  and media projection on the dart side of AndroidBackend.

- Implement those requests in MainActivity.kt and rework
  AndroidConnector to redirect those requests to activity.

- Automatically stop media projection when both sender and receiver
  are stopped. Prevent service from stopping projection while we
  are starting sender and receiver.

- Implement synchronization and fix various races.

- Use foreground service instead of bound service, to keep it
  running when app closes.

- Forbid swiping away notification on lock screen.
  (We can forbid it only for lock screen).

- Stop sender and receiver when notification is swiped away.

- Add AndroidListener, to pass events from kotlin to dart.
  Implement in in AndroidBackend on dart side.

- Add AndroidSenderSettings, AndroidReceiverSettings, and pass them
  from model to kotlin.

- Hard-code ports in model instead of kotlin.

- Implement Backend.getLocalAddresses().

- Improve comments.

- Refactor android service code.

- Remove unused values from strings.xml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gavv gavv

Labels
defect Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@gavv