react-native-bluetooth-classic icon indicating copy to clipboard operation
react-native-bluetooth-classic copied to clipboard

iOS Crash sometimes

Open cscovino opened this issue 3 years ago • 13 comments

Mobile Device Environment Provide a list of operating systems on which this issue is relevant.

  • Device: iPad
  • OS: iOS 15

Application Environment Provide information about your development environment:

  • React Native version: v0.64.1
  • RN Bluetooth Classic version: v1.60.0-rc.16

Describe the bug Sometimes after a while that I have connected the BT device, and I have sent and received data the app crashed. I think it could be a problem with memory because the device is always sending data and I see in logs that is constantly saying Stream %@ has bytes available. But this is my simple guess. The Bluetooth device is a LEGO Spike Prime.

The app crashed with this error and occurs in the DelimitedStringDeviceConnectionImpl.swift file line 237

AppStoreTools:       13A227
AppVariant:          1:iPad7,11:15
Beta:                YES
Code Type:           ARM-64 (Native)
Role:                unknown
Parent Process:      launchd [1]
Coalition:           es.robotix.c360 [569]

Date/Time:           2021-10-18 17:19:21.2074 -0300
Launch Time:         2021-10-18 16:12:47.2594 -0300
OS Version:          iPhone OS 15.0.2 (19A404)
Release Type:        User
Report Version:      104

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_INVALID_ADDRESS at 0x00004a63d2d1f290
Exception Codes: 0x0000000000000001, 0x00004a63d2d1f290
VM Region Info: 0x4a63d2d1f290 is not in any region.  Bytes after previous region: 81311562855057  
      REGION TYPE                 START - END      [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      commpage (reserved)     1000000000-7000000000 [384.0G] ---/--- SM=NUL  ...(unallocated)
--->  
      UNUSED SPACE AT END
Exception Note:  EXC_CORPSE_NOTIFY
Terminating Process: exc handler [481]
Triggered by Thread:  0

...

Thread 0 crashed with ARM Thread State (64-bit):
    x0: 0x00000002802ef2a0   x1: 0x000000018883fd00   x2: 0xfffffffffffffff3   x3: 0x0000000280fe9bb3
    x4: 0xfffffffffffa8e00   x5: 0x0000000000000020   x6: 0x00000002802f6990   x7: 0x0000000000000000
    x8: 0x00004a63d2d1f2a0   x9: 0xfffffffe00000000  x10: 0x0000000000000001  x11: 0x0000000100000028
   x12: 0x0000000000000000  x13: 0x6e22202c22626338  x14: 0x227274705f747865  x15: 0x0d7d7d323135203a
   x16: 0x00000001dc89a820  x17: 0x00000001855c5914  x18: 0x0000000000000000  x19: 0x000000016b526710
   x20: 0x000000016b526710  x21: 0x000000011bd34496  x22: 0x000000011bd34400  x23: 0x00000002802ef2a0
   x24: 0x0000000000000000  x25: 0x0000000104c90000  x26: 0x0000000104c8b000  x27: 0x00000001f11d5000
   x28: 0x00000001f11d5000   fp: 0x000000016b526660   lr: 0x0000000188265834
    sp: 0x000000016b526650   pc: 0x000000018883ef28 cpsr: 0x60000000
   esr: 0x92000004 (Data Abort) byte read Translation fault
Screen Shot 2021-10-21 at 19 22 18

cscovino avatar Oct 21 '21 22:10 cscovino

Hey, it's definitely possible - there hasn't been a lot of testing on the IOS side of things. My company/app doesn't use a stream of data like it seems you're device is sending.

  • Line 237 is definitely doing to keep growing inBuffer as long as the stream has available bytes.

I can only see two options:

  1. You're literally always sending data which means stream.hasBytesAvailable is always true and it never actually breaks out. This means that it never actually gets to line 240 which would empty the buffer.

  2. You're not listening for new data, nor are you manually requesting new data.

I'm going to guess, it's the former.

I'm not sure what the best way to handle this would be, there obviously needs to be a secondary break put in there. I sadly don't have a device available that would be able to test - that consistently sends data without breaks.

Max Bytes Per Read

Have a maximum amount of bytes that can be read at one time?

while (readBytes < 1024 && stream.hasBytesAvailable) {}

after 1024 (or whatever bytes) it would break out and allow the dataReceivedDelegate to take over.

Move the delegate INTO the while so that it happens all the time. This could cause a lot of extra work during the reading though.

Allow a flag for reading or not

Right now if the connection is established it will just automatically read from the stream. My initial thoughts were:

  • If you aren't exepcting data, don't be connected (which #1 above messes up if it never breaks)
  • but I guess we could add another flag to tell it to read or not, even if there is stuff available. But then the other end keeps sending.

Can you provide any more information??

  • Does this also happen on Android? The logic should be close to the same where a consistent stream of data will never break out.
  • How are you reading the data on the Javascript side? onRead or manually Reading? Can you attempt manually reading on a timer to see if it at least empties the buffer out?

IS this what you're using https://education.lego.com/en-us/products/lego-education-spike-prime-set/45678#spike%E2%84%A2-prime

Looks pretty cool, sadly I can't spend 400 bucks to help test this out with you.

kenjdavidson avatar Oct 21 '21 22:10 kenjdavidson

For future details : https://stackoverflow.com/questions/18677386/bluetooth-connection-to-lego-mindstorms-ev3-brick-from-ios-app

Mfi Protocol:

"COM.LEGO.MINDSTORMS.EV3"

kenjdavidson avatar Oct 21 '21 22:10 kenjdavidson

Yes, It is that. I know it's expensive 😅. I didn't test on Android because I don't need it at the moment.

It's option 1, yes. The device is always sending data to report its state (sensors, motors, display, gyroscope, etc.). It implements a JSON protocol like this.

I'm reading the data manually because I only take care of data when I send commands to the hub. So I think a timer that empties the buffer could be a solution. So I don't have to modify this library 😅.

cscovino avatar Oct 21 '21 23:10 cscovino

A good solution could be to implement the Max Bytes Per Read with the number as a config parameter.

cscovino avatar Oct 21 '21 23:10 cscovino

https://github.com/kenjdavidson/react-native-bluetooth-classic/blob/3dadaca202909a04fc43f61909ca6f345d824a28/android/src/main/java/kjd/reactnative/bluetooth/conn/DelimitedStringDeviceConnectionImpl.java#L55

It looks like the Android version does the message processing during the same step as adding data to the buffer. This is a pretty easy fix of copying lines 240-243 into the while().

kenjdavidson avatar Oct 21 '21 23:10 kenjdavidson

One of the things that didn't make sense to me is the stream programing on IOS (I'll be honest I can't really stand IOS development). But going back to the code it has this:

case .hasBytesAvailable:
    NSLog("Stream %@ has bytes available", aStream)
    readDataFromStream(aStream as! InputStream)
    break;

which then calls the readDataFromStream

while (stream.hasBytesAvailable) {
            let numBytesRead = stream.read(buffer, maxLength: readSize)
            
            if (numBytesRead < 0) {
                break;
            }
            
            inBuffer.append(buffer, count: numBytesRead)
        }

which repeatedly reads. The way I had it originally working was that:

  • The stream method (first block) called the read method
  • The read method read once and called the on Read delegate

Which my assumption was that the STREAM method would then find more bytes once that was done and continue the loop gain.

This is not the case and not how it works - or maybe my limited understanding of streams is not right.

I'm not sure if you do a lot of IOS programming, but if you know, I'd love to get your opinion. I think the problem when I was doing it, was the React Native does some funky things with the current thread:

runLoop: .currentRunloop didn't work runLoop: .main caused too much blocking and wasn't responding quickly enough

is why I think it is the way it is during my testing.

kenjdavidson avatar Oct 21 '21 23:10 kenjdavidson

If you can fork the project and play around with each one of these changes, let me know which is best then submit a PR, that would probably be the easiest way.

Rather than me guessing, having it work on my device (which doesn't stream consistently) and not working for you.

kenjdavidson avatar Oct 21 '21 23:10 kenjdavidson

No, I don't know much about iOS programming. Ok, I could test the solutions that you explain.

Thanks! 👍🏼

cscovino avatar Oct 22 '21 13:10 cscovino

You should be able to make the change locally, if you open up your project in XCODE, it should attach the Bluetooth classic library accordingly.

My first test would be to change:

    private func readDataFromStream(_ stream: InputStream) {
        let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: readSize)
        
        while (stream.hasBytesAvailable) {
            let numBytesRead = stream.read(buffer, maxLength: readSize)
            
            if (numBytesRead < 0) {
                break;
            }
            
            inBuffer.append(buffer, count: numBytesRead)
        }
    
        if let delegate = self.dataReceivedDelegate {
            while let data = read() {
                delegate.onReceivedData(fromDevice:  accessory, receivedData: data)
            }
        }
    }

to

    private func readDataFromStream(_ stream: InputStream) {
        let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: readSize)
        let numBytesRead = stream.read(buffer, maxLength: readSize)
        inBuffer.append(buffer, count: numBytesRead)
    
        if let delegate = self.dataReceivedDelegate {
            while let data = read() {
                delegate.onReceivedData(fromDevice:  accessory, receivedData: data)
            }
        }
    }

If that doesn't work, you might want to change:

            if let inStream = connected.inputStream,
                let outStream = connected.outputStream {
                inStream.delegate = self
                outStream.delegate = self
                inStream.schedule(in: .main, forMode: .commonModes)
                outStream.schedule(in: .main, forMode: .commonModes)
                inStream.open()
                outStream.open()
            }

to

            if let inStream = connected.inputStream,
                let outStream = connected.outputStream {
                inStream.delegate = self
                outStream.delegate = self
                inStream.schedule(in: .current, forMode: .currentMode)
                outStream.schedule(in: .current, forMode: .currentMode)
                inStream.open()
                outStream.open()
            }

or even:

                inStream.schedule(in: .current, forMode: .currentMode)
                outStream.schedule(in: .main, forMode: .default)

and see how that goes.

kenjdavidson avatar Oct 22 '21 13:10 kenjdavidson

Great! Thanks! 👍🏼

cscovino avatar Oct 22 '21 14:10 cscovino

Any luck with this?

I made the changes locally the other week but never released them.

If you confirm that's the issue (PR me) if you'd like me to commit and release the changes for you to try yourself, I don't mind doing it.

There are a limited number of people actually using the MFi side of this, so it shouldn't really affect anyone too badly.

kenjdavidson avatar Oct 29 '21 15:10 kenjdavidson

Sorry, I couldn't do the test. Because I hurried I went with the solution of creating a timer on js side.

When I test the options that you gave me, I will let you know. Probably next week.

cscovino avatar Oct 29 '21 16:10 cscovino

Ok, solid work around for now. Sorry about that. Glad you've got something working though.

kenjdavidson avatar Oct 29 '21 16:10 kenjdavidson