Skip to content

Add some missing unlocks#3893

Merged
sauwming merged 2 commits intopjsip:masterfrom
xhit:missing-unlocks
Mar 26, 2024
Merged

Add some missing unlocks#3893
sauwming merged 2 commits intopjsip:masterfrom
xhit:missing-unlocks

Conversation

@xhit
Copy link
Copy Markdown
Contributor

@xhit xhit commented Mar 21, 2024

Add some missing unlocks in the code.

Untested in local, I found some playing with the library, If some are unnecessary we can to edit.

nanangizz
nanangizz reviewed Mar 22, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@nanangizz nanangizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks.
Just a couple notes.

pjlib-util/src/pjlib-util/resolver.c Outdated
if (pkt->ans[0].name.slen >= PJ_MAX_HOSTNAME){
pj_grp_lock_release(resolver->grp_lock);
return PJ_ENAMETOOLONG;
}
Copy link
Copy Markdown
Member

@nanangizz nanangizz Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should maintain it as an assertion check, for example:

PJ_ASSERT_ON_FAIL(pkt->ans[0].name.slen < PJ_MAX_HOSTNAME, 
                  { pj_grp_lock_release(resolver->grp_lock); return PJ_ENAMETOOLONG; });

Copy link
Copy Markdown
Contributor Author

@xhit xhit Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ee6beea

pjlib-util/src/pjlib-util/resolver.c Outdated
pj_grp_lock_release(resolver->grp_lock);
return PJ_ENAMETOOLONG;
}

Copy link
Copy Markdown
Member

@nanangizz nanangizz Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Copy Markdown
Contributor

@wosrediinanatour wosrediinanatour Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea I head when seeing this PR: In other places a return is "deferred" in case of error:

//...

if (error) 
   goto on_error;

//...
return;

on_error:
   //... some cleanup ...

return;

This is usually quite useful with locks in C since there is nothing like RAII.

Copy link
Copy Markdown
Contributor Author

@xhit xhit Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ee6beea

@nanangizz nanangizz added this to the release-2.15 milestone Mar 22, 2024
@sauwming sauwming merged commit 427a2b3 into pjsip:master Mar 26, 2024
dshamaev-intermedia pushed a commit to intermedia-net/pjproject that referenced this pull request Apr 4, 2024
@xhit @dshamaev-intermedia
Add some missing unlocks (pjsip#3893)
e37d4e2
dshamaev-intermedia pushed a commit to intermedia-net/pjproject that referenced this pull request Apr 4, 2024
@xhit @dshamaev-intermedia
Add some missing unlocks (pjsip#3893)
ca3dba6
dshamaev-intermedia added a commit to intermedia-net/pjproject that referenced this pull request Apr 4, 2024
@github-actions @naf419
@goodicus @amubiera @xhit @sauwming @nanangizz @dshamaev-intermedia
Automated Pull Request (#77)
d2d79d1 
* Add missing openssl SECLEVEL=0 support (pjsip#3890)

Previous SECLEVEL support allowed for levels 1-5.
However, openssl defines levels 0-5. [1]

Recent openssl versions (3.0+) have moved previous
popular ciphers/key lengths (i.e. RSA1024withSHA1)
into level 0, so it is now a reasonable choice to use.

Add support for level 0.

[1] https://www.openssl.org/docs/man3.2/man3/SSL_CTX_set_security_level.html

* Enable Late Offer Answer Mode (LOAM) feature in the pjsua (pjsip#3869)

* Fix warnings for 32-bit compiler and misc fixes. (pjsip#3896)

* Add some missing unlocks (pjsip#3893)

* Prevent race condition in DTLS media stop (pjsip#3901)

* Fix data race reported by ThreadSanitizer in caching pool (pjsip#3897)

* Fixed Metal renderer memory leak (pjsip#3909)

* Fixed DTLS clock stoppage race (pjsip#3905)

* Improve IP address change IPv4 <-> IPv6 (pjsip#3910)

---------

Co-authored-by: naf <naf@sdf.org>
Co-authored-by: Goodicus <15110766+goodicus@users.noreply.github.com>
Co-authored-by: Amilcar Ubiera <chopin952@gmail.com>
Co-authored-by: Santiago De la Cruz <51337247+xhit@users.noreply.github.com>
Co-authored-by: sauwming <ming@teluu.com>
Co-authored-by: Nanang Izzuddin <nanang@teluu.com>
Co-authored-by: dshamaev-intermedia <105777082+dshamaev-intermedia@users.noreply.github.com>
dshamaev-intermedia added a commit to intermedia-net/pjproject that referenced this pull request Apr 5, 2024
@dshamaev-intermedia @nanangizz
@sauwming @xhit @andreas-wehrmann @trengginas
Feature/apps 17701 ipv6 and ip change fixes from 2.15 (#78)
83a228f 
* Add option to shutdown all transports on IP change (pjsip#3781)

* pjsua_handle_ip_change: Added missing null check for on_ip_changed_progress callback (pjsip#3830)

* Reset stored remote name in dialog (dlg->initial_dest) if transport is server. (pjsip#3783)

* Prevent immediate tsx termination upon transport error (pjsip#3805)

* Fixed issues when adding new media and deinitializing media (pjsip#3821)

* Potential issues when IPv6 is disabled (pjsip#3835)

* Improve IP address change IPv4 <-> IPv6 (pjsip#3910)

* Add some missing unlocks (pjsip#3893)

* add missing unlock (pjsip#3885)

* Retransmit 2xx response when transport is closed (pjsip#3828)

* Fixed account's route set update when modifying account (pjsip#3825)

---------

Co-authored-by: Nanang Izzuddin <nanang@teluu.com>
Co-authored-by: sauwming <ming@teluu.com>
Co-authored-by: Santiago De la Cruz <51337247+xhit@users.noreply.github.com>
Co-authored-by: Andreas Wehrmann <andreas-wehrmann@users.noreply.github.com>
Co-authored-by: Riza Sulistyo <trengginas@users.noreply.github.com>
dshamaev-intermedia added a commit to intermedia-net/pjproject that referenced this pull request Jun 12, 2024
@github-actions @naf419
@goodicus @amubiera @xhit @sauwming @nanangizz @dshamaev-intermedia @pespin @trengginas @peldszus
Automated Pull Request (#86)
e9b9e72 
* Add missing openssl SECLEVEL=0 support (pjsip#3890)

Previous SECLEVEL support allowed for levels 1-5.
However, openssl defines levels 0-5. [1]

Recent openssl versions (3.0+) have moved previous
popular ciphers/key lengths (i.e. RSA1024withSHA1)
into level 0, so it is now a reasonable choice to use.

Add support for level 0.

[1] https://www.openssl.org/docs/man3.2/man3/SSL_CTX_set_security_level.html

* Enable Late Offer Answer Mode (LOAM) feature in the pjsua (pjsip#3869)

* Fix warnings for 32-bit compiler and misc fixes. (pjsip#3896)

* Add some missing unlocks (pjsip#3893)

* Prevent race condition in DTLS media stop (pjsip#3901)

* Fix data race reported by ThreadSanitizer in caching pool (pjsip#3897)

* Fixed Metal renderer memory leak (pjsip#3909)

* Fixed DTLS clock stoppage race (pjsip#3905)

* Improve IP address change IPv4 <-> IPv6 (pjsip#3910)

* pjsua_acc: Fix warnings for comparison between ‘pjsua_nat64_opt’ and ‘enum pjsua_ipv6_use’ (pjsip#3915)

* Fix to ext_fmts accessed out of stack scope. (pjsip#3916)

* Add check in siprtp sample app for inactive audio media (pjsip#3927)

* Add function to initialize MediaFormat audio & video (pjsip#3925)

* Fixed incorrect SDP buffer length calculation (pjsip#3924)

* Support Push Notification in iOS sample app (pjsip#3913)

* Fixed PJSUA2 API to get/set Opus config (pjsip#3935)

* Fix bad address length check in pj_ioqueue_sendto(). (pjsip#3941)

* Fix warning of uninitialized value in fuzz-crypto (pjsip#3946)

* Print log on successful send (pjsip#3942)

* Fixed CI Mac build failure (pjsip#3947)

* Update Android JNI audio dev to use 16bit PCM only (pjsip#3945)

* Add TLS/SSL backend: Windows Schannel (pjsip#3867)

* pjsip_find_msg: Log warning if Content-Length field not found (pjsip#3960)

* Fix audiodev index (pjsip#3962)

* Fix assertion on call hangup from DTMF callback (pjsip#3970)

* Fix yaml error in github feature template (pjsip#3972)

* Fix version string in Python setup (pjsip#3976)

* Prevent pjmedia_codec_param.info.enc_ptime_denum division by zero in stream (pjsip#3975)

---------

Co-authored-by: naf <naf@sdf.org>
Co-authored-by: Goodicus <15110766+goodicus@users.noreply.github.com>
Co-authored-by: Amilcar Ubiera <chopin952@gmail.com>
Co-authored-by: Santiago De la Cruz <51337247+xhit@users.noreply.github.com>
Co-authored-by: sauwming <ming@teluu.com>
Co-authored-by: Nanang Izzuddin <nanang@teluu.com>
Co-authored-by: dshamaev-intermedia <105777082+dshamaev-intermedia@users.noreply.github.com>
Co-authored-by: CI Bot <noreply@intermedia.com>
Co-authored-by: Pau Espin Pedrol <pespin.shar@gmail.com>
Co-authored-by: Riza Sulistyo <trengginas@users.noreply.github.com>
Co-authored-by: Andreas Peldszus <andreas.peldszus@posteo.de>
BarryYin pushed a commit to BarryYin/pjproject that referenced this pull request Feb 3, 2026
@xhit
Add some missing unlocks (pjsip#3893)
6384202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sauwming sauwming sauwming approved these changes

@nanangizz nanangizz nanangizz approved these changes

@trengginas trengginas Awaiting requested review from trengginas

+1 more reviewer

@wosrediinanatour wosrediinanatour wosrediinanatour left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component: common type: bug

Projects

None yet

Milestone

release-2.15

Development

Successfully merging this pull request may close these issues.

4 participants

@xhit @sauwming @nanangizz @wosrediinanatour