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

Crash on NetSocket.swift #606

Closed
Goule opened this issue Oct 14, 2019 · 17 comments
Closed

Crash on NetSocket.swift #606

Goule opened this issue Oct 14, 2019 · 17 comments
Milestone

Comments

@Goule
Copy link
Contributor

Goule commented Oct 14, 2019

Hello,

Describe the bug
Crashlytics is reporting a crash on NetSocket.swift line 103 (NetSocket.doOutputProcess(_:maxLength:)).

To Reproduce
I can't reproduce it but it occured 10 times in 1 week in production (not many users).

Smartphone (please complete the following information):
Exemple of impacted devices :

  • 12.4.1 (16G102) / iPhone 7
  • 12.3.1 (16F203) / iPhone 6s
  • 12.4.2 (16G114) / iPad Air
  • 11.4.1 (15G77) / iPhone 6

Additional context
Here is the logs from Crashlytics :

Crashed: com.haishinkit.HaishinKit.NetSocket.output
0  CoreFoundation                 0x1d8cdd3e0 CFHash + 372
1  CoreFoundation                 0x1d8d70780 CFBasicHashGetCountOfKey + 204
2  CoreFoundation                 0x1d8cde8bc CFSetContainsValue + 116
3  CoreFoundation                 0x1d8cd6e18 CFRunLoopRemoveSource + 164
4  CFNetwork                      0x1d93ebc98 SocketStream::write(__CFWriteStream*, unsigned char const*, long, CFStreamError*) + 592
5  CoreFoundation                 0x1d8cec0c0 CFWriteStreamWrite + 300
6  HaishinKit                     0x10138dba4 NetSocket.doOutputProcess(_:maxLength:) + 103 (NetSocket.swift:103)
7  HaishinKit                     0x10138d98c closure #1 in NetSocket.doOutput(data:locked:) + 49 (NetSocket.swift:49)
8  HaishinKit                     0x1013355d8 thunk for @escaping @callee_guaranteed () -> () (<compiler-generated>)
9  libdispatch.dylib              0x1d8788a38 _dispatch_call_block_and_release + 24
10 libdispatch.dylib              0x1d87897d4 _dispatch_client_callout + 16
11 libdispatch.dylib              0x1d8732320 _dispatch_lane_serial_drain$VARIANT$mp + 592
12 libdispatch.dylib              0x1d8732e3c _dispatch_lane_invoke$VARIANT$mp + 428
13 libdispatch.dylib              0x1d873b4a8 _dispatch_workloop_worker_thread + 596
14 libsystem_pthread.dylib        0x1d8969114 _pthread_wqthread + 304
15 libsystem_pthread.dylib        0x1d896bcd4 start_wqthread + 4
crash_info_entry_1
*** CFHash() called with NULL ***

Have you any idea how to fix it ?
Thanks !

@floriangbh
Copy link

Hello,
Same issue for me, maybe this branch fixed the problem https://github.com/shogo4405/HaishinKit.swift/tree/feature/fix-crash-socket ?

@apolo2
Copy link

apolo2 commented Apr 9, 2020

i had the same issue. This happens when internet connection is very bad
You can set very bad network on developer tool on your phone to try

@mkrn
Copy link

mkrn commented May 5, 2020

Confirm. Happened going in and out of wifi zone, trying to reconnecting frequently. Will see if a catch can be added anywhere. Crash identical to @Goule

@francisykl-91
Copy link

I am facing the same issue, do we have an update on this?

@mkrn
Copy link

mkrn commented May 8, 2020

I've found a similar problem in this thread daltoniam/Starscream#422 The solution is to add a timer. I've been experimenting, and crash seems to happen less often. I can only reproduce when going in and out of bad network outside of the house.

However I will do additional testing with emulating bad network to try and catch it in debugger.

@wolfcon
Copy link
Contributor

wolfcon commented May 13, 2020

Hi, @mkrn How about changing Line 99 code just like this

// for outputStream as OutputStream
// true if the receiver can be written to or if a write must be attempted 
// in order to determine if space is available, false otherwise.
guard let outputStream = outputStream, outputStream.hasSpaceAvailable else {
    return
}
let length = outputStream.write(buffer.advanced(by: total), maxLength: maxLength - total)
if 0 < length {
    total += length
    totalBytesOut.mutate { $0 += Int64(length) }
    queueBytesOut.mutate { $0 -= Int64(length) }
} else if length == 0 {
    break;
}

Maybe outputStream cannot be written by some reason, check hasSpaceAvailable first, which according to:

true if the receiver can be written to or if a write must be attempted in order to determine if space is available, false otherwise.

@wolfcon
Copy link
Contributor

wolfcon commented May 14, 2020

This problem occurs when I reconnect.
Maybe these codes are caused the problem. OutputQueue async mission mess with disconnect method and reconnect method. Reconnect before disconnecting?

// NetSocket.swift
func close(isDisconnected: Bool) {
        outputQueue.async {
            guard self.runloop != nil else {
                return
            }
            self.deinitConnection(isDisconnected: isDisconnected)
        }
    }
// RTMPConnection
func close(isDisconnected: Bool) {
        guard connected || isDisconnected else {
            timer = nil
            return
        }
        if !isDisconnected {
            uri = nil
        }
        for (id, stream) in streams {
            stream.close()
            streams.removeValue(forKey: id)
        }
        socket.close(isDisconnected: false)
        timer = nil
        
        
    }

@mkrn
Copy link

mkrn commented Jun 4, 2020

It looks like the crash happens due to a threading issue.
My guess is reinitializing the queue this way in NetSocket:
outputQueue = .init(label: "com.haishinkit.HaishinKit.NetSocket.output", qos: qualityOfService)
Some previous tasks from the outputQueue are still executing (output of packets) and they try to access the same outputStream or executing the same block.
Looks like we need to add some additional code to handle it @wolfcon @shogo4405
I will attempt but my experience is limited.

@wolfcon
Copy link
Contributor

wolfcon commented Jun 4, 2020

rtmpConnection.requireNetworkFramework = true

@mkrn I have already used RTMPNWSocket to avoid this problem instead of RTMPSocket.
RTMPNWSocket is using Network.framework (>= iOS 12)

According to Apple suggestion:

Discouraged APIs
Foundation and SCNetworkReachability

+[NSStream getStreamsToHostWithName:port:inputStream:outputStream:]
+[NSStream getStreamsToHost:port:inputStream:outputStream:]
-[NSNetService getInputStream:outputStream:]

NSNetServiceListenForConnections
NSSocketPort
SCNetworkReachability

@mkrn
Copy link

mkrn commented Jun 4, 2020

@wolfcon 😲 so no more crashing?

@wolfcon
Copy link
Contributor

wolfcon commented Jun 5, 2020

@mkrn So far so good. No crash because of that.

@mkrn
Copy link

mkrn commented Jun 6, 2020

Thanks, @wolfcon This can be closed @shogo4405 .

mkrn added a commit to mkrn/HaishinKit.swift that referenced this issue Jun 16, 2020
shogo4405#606

- Do not interrupt recording on reconnect
@floriangbh
Copy link

floriangbh commented Jun 24, 2020

Hello,

Thanks for this solution !

I just tried. But since I put rtmpConnection.requireNetworkFramework = true my rtmpStatus handler is never called.
(self.rtmpConnection.addEventListener(.rtmpStatus, selector: #selector(rtmpStatusHandler), observer: self))
Is there extra modification when we use RTMPNWSocket ?

Thanks

@wolfcon
Copy link
Contributor

wolfcon commented Jun 29, 2020

@floriangbh No need any extra code. rtmpStatus is fine.

@floriangbh
Copy link

floriangbh commented Jun 29, 2020

Ok thanks @wolfcon !

But I just tried with the sample project and Youtube RTMP live stream, the sample doesn't work with the requireNetworkFramework option. Youtube Studio display an error about the format. :-(

@giotto2012
Copy link

@floriangbh If the protocol is RTMPS, please set it like this

rtmpConnection.requireNetworkFramework = true
let params = NWParameters.tls
rtmpConnection.parameters = params

@floriangbh
Copy link

@giotto2012 thanks that was my problem with RTMPS :)

@Goule Goule closed this as completed Jul 3, 2020
@shogo4405 shogo4405 added this to the 1.0.11 milestone Aug 16, 2020
mkrn added a commit to mkrn/HaishinKit.swift that referenced this issue Jul 26, 2021
shogo4405#606

- Do not interrupt recording on reconnect
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants