librtmp: Fix reset socket descriptor to -1 after closing RTMP connect…

[lilinxiong]

Description

1. The socket descriptor (sb_socket) is explicitly set to -1 after closing in the WriteN function.

2. The client ID cleanup logic in RTMP_Close is moved after the socket closure to maintain proper resource cleanup order.

Motivation and Context

#03 pc 000000000045d7cc /lib/arm64/liblive-push.so (RTMPSockBuf_Close+112) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#04 pc 000000000045d56c /lib/arm64/liblive-push.so (WriteN+420) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#05 pc 000000000045c4cc /lib/arm64/liblive-push.so (RTMP_SendPacket+1964) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#06 pc 0000000000459e48 /lib/arm64/liblive-push.so (SendFCUnpublish+264) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#07 pc 0000000000456e2c /lib/arm64/liblive-push.so (RTMP_Close+196) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#08 pc 000000000045d574 /lib/arm64/liblive-push.so (WriteN+428) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#09 pc 000000000045c4cc /lib/arm64/liblive-push.so (RTMP_SendPacket+1964) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)
#10 pc 000000000045f0d0 /lib/arm64/liblive-push.so (RTMP_Write+744) (BuildId: 7847e45a2319a0441b147ccfbdda6841f007740f)

I was using RTMP for live streaming on Android, and when I shut down the server, the above stack trace appeared. After investigation, the cause was as follows:

1. The immediate cause is that Android prohibits calling close() on a socket that has already been closed.
2. In RTMP_Close(), if the current sb_socket is still valid, the flow re-enters WriteN() via SendFCUnpublish() -> RTMP_SendPacket(). This causes RTMPSockBuf_Send() to fail again, leading back to RTMP_Close() and creating an infinite loop.

How Has This Been Tested?

Types of changes

Checklist:

• [x] My code has been run through clang-format.
• [x] I have read the contributing document.
• [x] My code is not on the master branch.
• [x] The code has been tested.
• [x] All commit messages are properly formatted and commits squashed where appropriate.
• [x] I have included updates to all appropriate documentation.

[norihiro]

Please remove the punctuation (.) from the commit message title. (You can edit it by git commit --amend and force push to GitHub.)

[lilinxiong]

Please remove the punctuation (.) from the commit message title. (You can edit it by git commit --amend and force push to GitHub.)

done.

[notr1ch]

Wouldn't doing this in RTMPSockBuf_Close be a more appropriate place?

[lilinxiong]

Wouldn't doing this in RTMPSockBuf_Close be a more appropriate place?

You are right.

[Fenrirthviti]

Out of curiosity, where exactly is that stack from? I'm unable to find any references to a liblive-push anywhere. Is this something based on librtmp?

[lilinxiong]

Out of curiosity, where exactly is that stack from? I'm unable to find any references to a liblive-push anywhere. Is this something based on librtmp?

liblive-push is a library we use in our own project for live streaming, and yes, it is based on librtmp.

[Fenrirthviti]

Is this something that you are distributing? Is the source available? I'm reluctant to accept a fix based on a project that might be violating the librtmp license without understanding the context a bit more. Thanks in advance.

[lilinxiong]

Is this something that you are distributing? Is the source available? I'm reluctant to accept a fix based on a project that might be violating the librtmp license without understanding the context a bit more. Thanks in advance.

Since it’s my own demo project and it’s not fully developed yet, it will be available on GitHub later with a public repository. If it’s absolutely necessary to wait until my demo project is fully open-sourced before merging, it will likely take 1 to 3 weeks, as I can only work on it during my spare time. However, this bug is a logical error.

[Fenrirthviti]

That should be fine then, no need to delay for your release if it is just internal for the moment. Thanks for the context, we were just curious about the crash stack and couldn't find the library anywhere.

[lilinxiong]

That should be fine then, no need to delay for your release if it is just internal for the moment. Thanks for the context, we were just curious about the crash stack and couldn't find the library anywhere.

So can it be merged normally now?