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

menu: fix segfault in ua_event_handler (#1059) #1061

Merged
merged 1 commit into from Aug 15, 2020

Conversation

cspiel1
Copy link
Collaborator

@cspiel1 cspiel1 commented Aug 14, 2020

A segfault occurs in modules loaded after menu and have an ua_event_handler
that use call object (to collect information about the call). The call object
may not be de-referenced in any ua_event_handler.

@alfredh
Copy link
Collaborator

alfredh commented Aug 14, 2020

thanks for the patch.

the patch fixes the problem, but there is one thing that I think we should avoid.

call.c calls mem_deref(call), in general it is bad practice and dangerous to destroy a "self" object
inside the object's code. mem_deref should be done on the outside.

a better solution is to add a call_close() function that closes the call object,
but does not destroy it. sample code:

void call_close(struct call *call)
{
	if (!call)
		return;

	list_unlink(&call->le);
	tmr_cancel(&call->tmr_dtmf);

	call->sess = mem_deref(call->sess);

	set_state(call, CALL_STATE_TERMINATED);

	etc ...
}

this function can then be called from ua.c

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 15, 2020

Hi Alfred,

you are right! The call_destructor() does all these things you suggest for call_close(). Maybe the video_error_handler() shows the solution. That's how it was done before the problematic commits mentioned in #1059.

I will push again.

r,c

A segfault occurs in modules loaded after menu and have an ua_event_handler
that use call object (to collect information about the call). The call object
may not be de-referenced in any ua_event_handler.
@alfredh
Copy link
Collaborator

alfredh commented Aug 15, 2020

is this the final code that you want merged ?

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 15, 2020

Yes, if it's ok for you.

@alfredh alfredh merged commit f420218 into baresip:master Aug 15, 2020
@alfredh
Copy link
Collaborator

alfredh commented Aug 15, 2020

yes, great. Thanks for fixing the bug!

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 15, 2020

We have much more. ;-) Thanks for review and merging!

@cspiel1 cspiel1 deleted the bugfix/#1059 branch October 21, 2020 05:52
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Nov 28, 2020
(Presumably the changelog [Unreleased] section is for 1.0.0)

= Baresip Changelog

== [Unreleased]

=== Added

- aac: add AAC_STREAMTYPE_AUDIO enum value
- aac: add AAC_ prefix
- Video mode param to call_answer(), ua_answer() and ua_hold_answer [#966]
- video_stop_display() API function [#977]
- module: add path to module_load() function
- conf: add conf_configure_buf
- test: add usage of g711.so module [#978]
- JSON initial codec state command and response [#973]
- account_set_video_codecs() API function [#981]
- net: add fallback dns nameserver [#996]
- gtk: show call_peername in notify title [#1006]
- call: Added call_state() API function that returns enum state of the call [#1013]
- account_set_stun_user() and account_set_stun_pass() API functions [#1015]
- API functions account_stun_uri and account_set_stun_uri. [#1018]
- ausine: Audio sine wave input module [#1021]
- gtk/menu: replace spaces from uri [#1007]
- jack: allowing jack client name to be specified in the config file [#1025] [#1020]
- snapshot: Add snapshot_send and snapshot_recv commands [#1029]
- webrtc_aec: 'extended_filter' config option [#1030]
- avfilter: FFmpeg filter graphs integration [#1038]
- reg: view proxy expiry value in reg_status [#1068]
- account: add parameter rwait for re-register interval [#1069]
- call, stream, menu: add cmd to set the direction of video stream [#1073]

=== Changed

- **Using [baresip/re](https://github.com/baresip/re) fork now**
- audio: move calculation to audio_jb_current_value
- avformat: clean up docs
- gzrtp: update docs
- account: increased size of audio codec list to 16
- video: make video_sdp_attr_decode public
- config: Derive default audio driver from default audio device [#1009]
- jack: modifying info message on jack client creation [#1019]
- call: when video stream is disabled, stop also video display [#1023]
- dtls_srtp: use tls_set_selfsigned_rsa with keysize 2048 [#1062] [#1056]
- rst: use a min ptime of 20ms
- aac: change ptime to 4ms

=== Fixed

- avcodec: fix H.264 interop with Firefox
- winwave: waveInGetPosition is no longer supported for use as of Windows Vista [#960]
- avcodec: call av_hwdevice_ctx_create before if-statement
- account: use single quote instead of backtick
- ice: fix segfault in connh [#980]
- call: Update call->got_offer when re-INVITE or answer to re-INVITE
  is received [#986]
- mk: Test also for /usr/lib64/libspeexdsp.so to cover Fedora/RHEL/CentOS [#992]
- config: Allow distribution specific CA trust bundle locations (fixes [#993]
- config: Allow distribution specific default audio device (fixes [#994]
- mqtt: fix err is never read (found by clang static analyzer)
- avcodec: fix err is never read (found by clang static analyzer)
- gtk: notification buttons do not work on Systems [#1012]
- gtk: fix dtmf_tone and add tones as feedback [#1010]
- pulse: drain pulse buffers before freeing [#1016]
- jack: jack_play connect all physical ports [#1028]
- Makefile: do not try to install modules if build is static [#1031]
- gzrtp: media_alloc function is missing [#1034] [#1022]
- call: when updating video, check if video stream has been disabled [#1037]
- amr: fix length check, fixes [#1011]
- modules: fix search path for avdevice.h [#1043]
- gtk: declare variables C89 style
- config: init newly added member
- menu: fix segfault in ua_event_handler [#1059] [#1061]
- debug_cmd: fix OpenSSL no-deprecated [#1065]
- aac: handle missing bitrate parameter in SDP format
- av1: properly configure encoder

=== Removed

- ice: remove support for ICE-lite
- ice: remove ice_debug, use log level DEBUG instead
- ice: make stun server optional
- config: remove ice_debug option (unused)

=== Contributors (many thanks)

- Alfred E. Heggestad
- Alexander Gramner
- Andrew Webster
- Christian Spielberger
- Christoph Huber
- Davide Alberani
- Ethan Funk
- Juha Heinanen
- mbattista
- Michael Malone
- Mikl Kurkov
- ndilieto
- Robert Scheck
- Roger Sandholm
- Sebastian Reimers

[#966]: baresip/baresip#966
[#977]: baresip/baresip#977
[#978]: baresip/baresip#978
[#973]: baresip/baresip#973
[#981]: baresip/baresip#981
[#996]: baresip/baresip#996
[#1006]: baresip/baresip#1006
[#1013]: baresip/baresip#1013
[#1015]: baresip/baresip#1015
[#1018]: baresip/baresip#1018
[#1021]: baresip/baresip#1021
[#1007]: baresip/baresip#1007
[#1025]: baresip/baresip#1025
[#1020]: baresip/baresip#1020
[#1029]: baresip/baresip#1029
[#1030]: baresip/baresip#1030
[#1038]: baresip/baresip#1038
[#1009]: baresip/baresip#1009
[#1019]: baresip/baresip#1019
[#1023]: baresip/baresip#1023
[#1062]: baresip/baresip#1062
[#1056]: baresip/baresip#1056
[#960]: baresip/baresip#960
[#980]: baresip/baresip#980
[#986]: baresip/baresip#986
[#992]: baresip/baresip#992
[#993]: baresip/baresip#993
[#994]: baresip/baresip#994
[#1012]: baresip/baresip#1012
[#1010]: baresip/baresip#1010
[#1016]: baresip/baresip#1016
[#1028]: baresip/baresip#1028
[#1031]: baresip/baresip#1031
[#1034]: baresip/baresip#1034
[#1022]: baresip/baresip#1022
[#1037]: baresip/baresip#1037
[#1011]: baresip/baresip#1011
[#1043]: baresip/baresip#1043
[#1059]: baresip/baresip#1059
[#1061]: baresip/baresip#1061
[#1065]: baresip/baresip#1065
[#1068]: baresip/baresip#1068
[#1069]: baresip/baresip#1069
[#1073]: baresip/baresip#1073

[Unreleased]: baresip/baresip@v0.6.6...HEAD
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

Successfully merging this pull request may close these issues.

None yet

2 participants