7:57 p.m.
---
gatchat/gatppp.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 1d41ded..d9b1627 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -446,6 +446,9 @@ void g_at_ppp_unref(GAtPPP *ppp)
{
gboolean is_zero;
+ if (ppp == NULL)
+ return;
+
is_zero = g_atomic_int_dec_and_test(&ppp->ref_count);
if (is_zero == FALSE)
--
1.6.3.3
7:57 p.m.
New subject: [PATCH 2/2] gathdlc: Unset disconnect function in unref
If we destroy PPP instance from g_at_ppp_unref, we should not invoke
io disconnect function.
---
gatchat/gathdlc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gatchat/gathdlc.c b/gatchat/gathdlc.c
index 08a1939..0186e46 100644
--- a/gatchat/gathdlc.c
+++ b/gatchat/gathdlc.c
@@ -278,6 +278,7 @@ void g_at_hdlc_unref(GAtHDLC *hdlc)
hdlc->record_fd = -1;
}
+ g_at_io_set_disconnect_function(hdlc->io, NULL, NULL);
g_at_io_unref(hdlc->io);
hdlc->io = NULL;
--
1.6.3.3
8:03 p.m.
New subject: [PATCH 2/2] gathdlc: Unset disconnect function in unref
Hi,
Zhenhua Zhang wrote:
If we destroy PPP instance from g_at_ppp_unref, we should not invoke
io disconnect function. ---
gatchat/gathdlc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gatchat/gathdlc.c b/gatchat/gathdlc.c
index 08a1939..0186e46 100644
--- a/gatchat/gathdlc.c
+++ b/gatchat/gathdlc.c
@@ -278,6 +278,7 @@ void g_at_hdlc_unref(GAtHDLC *hdlc)
hdlc->record_fd = -1; }
+ g_at_io_set_disconnect_function(hdlc->io, NULL, NULL);
g_at_io_unref(hdlc->io); hdlc->io = NULL;
Here the problem is that disconnect function could be invoked if remote IO is disconnected
after our ppp instance is freed. So we should avoid to invoke io_disconnect() in below
case.
Entering new phase: 5
PPP: lcp: pppcp_timeout: current state 5:STOPPING
PPP: lcp: pppcp_generate_event: current state 5:STOPPING
PPP: event: 5 (TO-), action: 803, new_state: 3 (STOPPED)
PPP: lcp: pppcp_this_layer_finished: current state 3:STOPPED
Entering new phase: 0
ofonod[3114]: src/emulator.c:ppp_disconnect()
Server: > \r\nNO CARRIER\r\n
ofonod[3114]: Pcui:< \r\n^BOOT:20291346,0,0,0,20\r\n
Server: < AT+CFUN=0\r
ofonod[3114]: src/emulator.c:cfun_cb() set CFUN to 0
Server: > \r\nOK\r\n
(gdb) bt
#0 0x0806faf6 in pppcp_generate_event (data=0x95bfde0, event_type=DOWN, packet=0x0,
len=0)
at gatchat/ppp_cp.c:638
#1 0x0806e9cd in io_disconnect (user_data=0x95bfde8) at gatchat/gatppp.c:349
#2 0x001ea504 in g_source_callback_unref (cb_data=0x95a8678)
at /build/buildd/glib2.0-2.22.3/glib/gmain.c:1077
#3 0x001eaace in g_source_destroy_internal (source=0x95a86b0, context=<value optimized
out>, have_lock=1)
at /build/buildd/glib2.0-2.22.3/glib/gmain.c:856
#4 0x001eaedb in g_main_dispatch (context=0x95a5f80) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:1985
#5 IA__g_main_context_dispatch (context=0x95a5f80) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:2513
#6 0x001ee730 in g_main_context_iterate (context=0x95a5f80, block=<value optimized
out>, dispatch=1,
self=0x95a6c58) at /build/buildd/glib2.0-2.22.3/glib/gmain.c:2591
#7 0x001eeb9f in IA__g_main_loop_run (loop=0x95a5ef0) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:2799
#8 0x08094548 in main (argc=1, argv=0xbffb8ba4) at src/main.c:227
(gdb) frame 1
#1 0x0806e9cd in io_disconnect (user_data=0x95bfde8) at gatchat/gatppp.c:349
349 pppcp_signal_down(ppp->lcp);
(gdb) p ppp
$6 = <value optimized out>
(gdb) info local
No locals.
(gdb)
--
1.6.3.3
_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono
Regards,
Zhenhua
2:45 a.m.
New subject: [PATCH 2/2] gathdlc: Unset disconnect function in unref
Hi Zhenhua,
Here the problem is that disconnect function could be invoked if
remote IO is disconnected after our ppp instance is freed. So we should avoid to invoke
io_disconnect() in below case.
Entering new phase: 5
PPP: lcp: pppcp_timeout: current state 5:STOPPING
PPP: lcp: pppcp_generate_event: current state 5:STOPPING
PPP: event: 5 (TO-), action: 803, new_state: 3 (STOPPED)
PPP: lcp: pppcp_this_layer_finished: current state 3:STOPPED
Entering new phase: 0
ofonod[3114]: src/emulator.c:ppp_disconnect()
Server: > \r\nNO CARRIER\r\n
ofonod[3114]: Pcui:< \r\n^BOOT:20291346,0,0,0,20\r\n
Server: < AT+CFUN=0\r
ofonod[3114]: src/emulator.c:cfun_cb() set CFUN to 0
Server: > \r\nOK\r\n
(gdb) bt
#0 0x0806faf6 in pppcp_generate_event (data=0x95bfde0, event_type=DOWN, packet=0x0,
len=0)
at gatchat/ppp_cp.c:638
#1 0x0806e9cd in io_disconnect (user_data=0x95bfde8) at gatchat/gatppp.c:349
#2 0x001ea504 in g_source_callback_unref (cb_data=0x95a8678)
at /build/buildd/glib2.0-2.22.3/glib/gmain.c:1077
#3 0x001eaace in g_source_destroy_internal (source=0x95a86b0, context=<value
optimized out>, have_lock=1)
at /build/buildd/glib2.0-2.22.3/glib/gmain.c:856
#4 0x001eaedb in g_main_dispatch (context=0x95a5f80) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:1985
#5 IA__g_main_context_dispatch (context=0x95a5f80) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:2513
#6 0x001ee730 in g_main_context_iterate (context=0x95a5f80, block=<value optimized
out>, dispatch=1,
self=0x95a6c58) at /build/buildd/glib2.0-2.22.3/glib/gmain.c:2591
#7 0x001eeb9f in IA__g_main_loop_run (loop=0x95a5ef0) at
/build/buildd/glib2.0-2.22.3/glib/gmain.c:2799
#8 0x08094548 in main (argc=1, argv=0xbffb8ba4) at src/main.c:227
(gdb) frame 1
#1 0x0806e9cd in io_disconnect (user_data=0x95bfde8) at gatchat/gatppp.c:349
349 pppcp_signal_down(ppp->lcp);
(gdb) p ppp
$6 = <value optimized out>
(gdb) info local
No locals.
(gdb)
It seems the real culprit is that GAtServer is not restoring the
io_disconnect on the GAtIO object in g_at_server_resume. Why are we not
seeing this crash in test-server?
Regards,
-Denis
2:28 a.m.
New subject: [PATCH 2/2] gathdlc: Unset disconnect function in unref
Hi Zhenhua,
@@ -278,6 +278,7 @@ void g_at_hdlc_unref(GAtHDLC *hdlc)
hdlc->record_fd = -1;
}
+ g_at_io_set_disconnect_function(hdlc->io, NULL, NULL);
Since GAtHDLC does not set the disconnect function, this really belongs
in GAtPPP.
g_at_io_unref(hdlc->io);
hdlc->io = NULL;
However, I really question why this patch is necessary. The only way
the disconnect function is not reset today is if the GAtIO is refed and
g_at_chat_resume is not called.
We do call g_at_io_set_disconnect function in g_at_chat_resume.
Regards,
-Denis
11:32 a.m.
New subject: [PATCH 2/2] gathdlc: Unset disconnect function in unref
Hi Denis,
Denis Kenzior wrote:
Hi Zhenhua,
> @@ -278,6 +278,7 @@ void g_at_hdlc_unref(GAtHDLC *hdlc)
> hdlc->record_fd = -1; }
>
> + g_at_io_set_disconnect_function(hdlc->io, NULL, NULL);
Since GAtHDLC does not set the disconnect function, this really
belongs in GAtPPP.
How about unset io_disconnect() in GAtPPP? Since GAtPPP doesn't have suspend/resume,
the io_disconnect always exist until ppp is dead.
> g_at_io_unref(hdlc->io);
> hdlc->io = NULL;
>
However, I really question why this patch is necessary. The only way
the disconnect function is not reset today is if the GAtIO is refed
and g_at_chat_resume is not called.
We do call g_at_io_set_disconnect function in g_at_chat_resume.
Good catch. I fixed it in gatserver and will send a patch out soon. Yes, after reset the
disconnect function, the problem is gone.
Regards,
-Denis
Regards,
Zhenhua
10:48 p.m.
Hi Zhenhua,
---
gatchat/gatppp.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 1d41ded..d9b1627 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -446,6 +446,9 @@ void g_at_ppp_unref(GAtPPP *ppp)
{
gboolean is_zero;
+ if (ppp == NULL)
+ return;
+
is_zero = g_atomic_int_dec_and_test(&ppp->ref_count);
if (is_zero == FALSE)
since we have been safe-guarding this in other unref functions as well,
I also applied this patch.
In general the calling code is doing something wrong here if it tries to
unref a NULL pointer.
Regards
Marcel
4456
days inactive
4457
days old
6 comments
4 participants
participants (4)
-
Denis Kenzior -
Marcel Holtmann -
Zhang, Zhenhua -
Zhenhua Zhang