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

Wrong return value in janus_mutex_trylock for PTHREAD implementation #2894

Closed
BogdanovKirill opened this issue Feb 18, 2022 · 3 comments
Closed

Comments

@BogdanovKirill
Copy link

Bug: contract of janus_mutex_trylock depends on compilation settings (pthread enabled/disabled)

#define janus_mutex_trylock_nodebug(a) { ret = pthread_mutex_trylock(a); }

Ассording to manual for pthread_mutex_trylock :
"If successful, the pthread_mutex_lock() and pthread_mutex_unlock() functions shall return zero; otherwise, an error number shall be returned to indicate the error."

It means that you should logically invert result value:
ret = !pthread_mutex_trylock(a);

Also you could compare with GLIB implementation:

#define janus_mutex_trylock_nodebug(a) { ret = g_mutex_trylock(a); }

Ассording to manual for g_mutex_trylock :
Tries to lock mutex. If mutex is already locked by another thread, it immediately returns FALSE. Otherwise it locks mutex and returns TRUE.

@lminiero
Copy link
Member

Good catch. It looks like we currently only use it once, in the Streaming plugin, where we assume the Glib logic you pasted is the correct one. As such, yes, as you suggest we should invert the pthread definition. I'll do that shortly.

@BogdanovKirill
Copy link
Author

@lminiero
It seems that your fix is not enough. Check here:

#define janus_mutex_trylock_debug(a) { JANUS_PRINT("[%s:%s:%d:trylock] %p\n", __FILE__, __FUNCTION__, __LINE__, a); ret = pthread_mutex_trylock(a); }

@lminiero
Copy link
Member

Oh right, the debug/nodebug variants... will fix them too 👍

lminiero added a commit that referenced this issue Feb 18, 2022
lminiero added a commit that referenced this issue Feb 18, 2022
mwalbeck pushed a commit to mwalbeck/docker-janus-gateway that referenced this issue Mar 16, 2022
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [meetecho/janus-gateway](https://github.com/meetecho/janus-gateway) | major | `v0.11.7` -> `v1.0.0` |

---

### Release Notes

<details>
<summary>meetecho/janus-gateway</summary>

### [`v1.0.0`](https://github.com/meetecho/janus-gateway/blob/HEAD/CHANGELOG.md#v100---2022-03-03)

[Compare Source](meetecho/janus-gateway@v0.12.0...v1.0.0)

-   Refactored Janus to support multistream PeerConnections \[[PR-2211](meetecho/janus-gateway#2211)]
-   Moved all source files under new 'src' folder to unclutter the repo \[[PR-2885](meetecho/janus-gateway#2885)]
-   Fixed definition of trylock wrapper when using pthreads \[[Issue-2894](meetecho/janus-gateway#2894)]
-   Fixed broken RTP when no extensions are negotiated
-   Added checks when inserting RTP extensions to avoid buffer overflows
-   Added missing support for disabled rid simulcast substreams in SDP \[[PR-2888](meetecho/janus-gateway#2888)]
-   Fixed TWCC feedback when simulcast SSRCs are missing (thanks [@&#8203;OxleyS](https://github.com/OxleyS)!) \[[PR-2908](meetecho/janus-gateway#2908)]
-   Added support for playout-delay RTP extension \[[PR-2895](meetecho/janus-gateway#2895)]
-   Fixed partially broken H.264 support when using Firefox in VideoRoom
-   Fixed new VideoRoom rtp_forward API ignoring some properties
-   Fixed deadlock and segfault when stopping Streaming mountpoint recordings \[[Issue-2902](meetecho/janus-gateway#2902)]
-   Fixed RTSP support in Streaming plugin for cameras that expect path-only DESCRIBE requests (thanks [@&#8203;jp-bennett](https://github.com/jp-bennett)!) \[[PR-2909](meetecho/janus-gateway#2909)]
-   Fixed RTP being relayed incorrectly in Lua and Duktape plugins
-   Added Duktape as optional dependency, instead of embedding the engine code \[[PR-2886](meetecho/janus-gateway#2886)]
-   Fixed crash at startup when not able to connect to RabbitMQ server
-   Improved fuzzing and checks on RTP extensions
-   Removed distinction between simulcast and simulcast2 in janus.js \[[PR-2887](meetecho/janus-gateway#2887)]
-   Other smaller fixes and improvements (thanks to all who contributed pull requests and reported issues!)

### [`v0.12.0`](meetecho/janus-gateway@v0.11.8...v0.12.0)

[Compare Source](meetecho/janus-gateway@v0.11.8...v0.12.0)

### [`v0.11.8`](https://github.com/meetecho/janus-gateway/blob/HEAD/CHANGELOG.md#v0118---2022-02-11)

[Compare Source](meetecho/janus-gateway@v0.11.7...v0.11.8)

-   Added initial (and limited) integration of RED audio \[[PR-2685](meetecho/janus-gateway#2685)]
-   Added support for Two-Byte header RTP extensions (RFC8285) and, partially, for the new Depencency Descriptor RTP extension (needed for AV1-SVC) \[[PR-2741](meetecho/janus-gateway#2741)]
-   Fixed rare race conditions between sending a packet and closing a connection \[[PR-2869](meetecho/janus-gateway#2869)]
-   Fix last stats before closing PeerConnection not being sent to handlers (thanks [@&#8203;zodiak83](https://github.com/zodiak83)!) \[[PR-2874](meetecho/janus-gateway#2874)]
-   Changed automatic allocation on static loops from round robin to least used \[[PR-2878](meetecho/janus-gateway#2878)]
-   Added new API to bulk start/stop MJR-based recordings in AudioBridge \[[PR-2862](meetecho/janus-gateway#2862)]
-   Fixed broken duration in spatial AudioBridge recordings
-   Fixed broken G.711 RTP forwarding in AudioBridge (thanks [@&#8203;AlexYaremchuk](https://github.com/AlexYaremchuk)!) \[[PR-2875](meetecho/janus-gateway#2875)]
-   Fixed broken recordings in NoSIP plugin
-   Fixed warnings when postprocessing Opus recordings with DTX packets
-   Other smaller fixes and improvements (thanks to all who contributed pull requests and reported issues!)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-janus-gateway/pulls/65
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants