Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
- From: Dan Williams <dcbw redhat com>
- To: Ben Chan <benchan chromium org>
- Cc: networkmanager-list gnome org
- Subject: Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed
- Date: Thu, 29 Nov 2012 17:42:52 -0600
On Thu, 2012-11-29 at 11:37 -0800, Ben Chan wrote:
> The crash happens to be a reuse of an already disposed MMSerialPort
> object. I'm now tracing the new/dispose paths.
Can you toss a mm_info() into the mm-base-modem.c's dispose() function?
Also one into mm_device_remove_modem().
It looks like the modem is getting disposed but then something is
causing it to get enabled again after wakeup:
ModemManager[2858]: <debug> [1354146967.597474] [mm-base-modem.c:1252]
finalize(): Modem (Samsung) '/sys/devices/s5p-ehci/usb1/1-2/1-2.1'
completely disposed
ModemManager[2858]: <info> [1354147018.549702]
[mm-broadband-modem.c:7341] enabling_step(): Modem enabling...
ModemManager[2858]: <info> [1354147018.549838] [mm-iface-modem.c:1176]
mm_iface_modem_update_state():
Modem /org/freedesktop/ModemManager1/Modem/1: state changed (disabled ->
enabling)
Problems actually start earlier here:
ModemManager[2858]: <info> [1354146961.534481] [mm-iface-modem.c:1176]
mm_iface_modem_update_state():
Modem /org/freedesktop/ModemManager1/Modem/1: state changed (disabling
-> disabled)
ModemManager[2858]: <info> [1354146967.576974] [mm-serial-port.c:857]
mm_serial_port_open(): (ttyACM1) opening serial port...
ModemManager[2858]: <debug> [1354146967.580958] [mm-serial-port.c:926]
mm_serial_port_open(): (ttyACM1) device open count is 1 (open)
(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion
`G_IS_OBJECT (object)' failed
would be nice to know what object that is, but the only good way of
debugging this is to be in gdb and break on 'g_log' and then get a
backtrace. not very fun, though you could run without --debug and
reduce the number of times you have to hit "c<return>".
In any case, we have to figure out why the modem is getting disposed,
but apparently isn't fully disposed, and also apparently isn't
un-exported from the bus. That's what mm_device_remove_modem() should
be doing for us.
Dan
>
> Ben
>
>
> /* Don't read any input if the current command isn't done being
> sent yet */
> info = g_queue_peek_nth (priv->queue, 0);
> 49f60: 6940 ldr r0, [r0, #20]
> <- 0x151F60
> 49f62: f7c9 ebf6 blx 13750 <_init+0x1254>
>
>
> ==20047== Invalid read of size 4
> ==20047== at 0x151F60: ??? (in /usr/sbin/ModemManager)
> ==20047== Address 0x14 is not stack'd, malloc'd or (recently) free'd
> ==20047==
> ==20047==
> ==20047== Process terminating with default action of signal 11
> (SIGSEGV)
> ==20047== Access not within mapped region at address 0x14
> ==20047== at 0x151F60: ??? (in /usr/sbin/ModemManager)
> ==20047== If you believe this happened as a result of a stack
> ==20047== overflow in your program's main thread (unlikely but
> ==20047== possible), you can try to increase the size of the
> ==20047== main thread stack using the --main-stacksize= flag.
> ==20047== The main thread stack size used in this run was 8388608.
>
>
> (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion
> `G_IS_OBJECT (object)' failed
> (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion
> `G_IS_OBJECT (object)' failed
> (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref:
> assertion `G_IS_OBJECT (object)' failed
> (ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref:
> assertion `G_IS_OBJECT (object)' failed
> (ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance
> `0x78624028' has no handler with id `148'
> (ModemManager:2858): GLib-GObject-WARNING **: invalid unclassed
> pointer in cast to `MMSerialPort'
> (ModemManager:2858): GLib-GObject-CRITICAL **:
> g_type_instance_get_private: assertion `instance != NULL &&
> instance->g_class != NULL' failed
>
> On Thu, Nov 29, 2012 at 11:26 AM, Dan Williams <dcbw redhat com>
> wrote:
> On Tue, 2012-11-27 at 13:19 -0800, Ben Chan wrote:
> > Yes, it's related to the data_available
> (mm-serial-port.c:767) crash
> > (crosbug.com/35391). I'm running suspend/resume stress test
> with
> > ModemManager under valgrind.
>
>
> Any way to get MM running with --debug in those logs? Also,
> in your
> sources, does line 767 match up with:
>
> info = g_queue_peek_nth (priv->queue, 0);
>
> or some other line?
>
> One other thing to do, put an
>
> mm_info ("(%s): disposing", mm_port_get_device (MM_PORT
> (self)));
>
> into dispose() and lets see when it gets disposed and if
> anything tries
> to open it during/after dispose. Drop another of these into
> mm_serial_port_new().
>
> Dan
>
> >
> > Thanks,
> > Ben
> >
> > On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado
> > <aleksander lanedo com> wrote:
> > On 11/27/2012 09:39 PM, Ben Chan wrote:
> > > ---
> > > src/mm-serial-port.c | 10 ++++++++++
> > > 1 files changed, 10 insertions(+), 0 deletions(-)
> >
> >
> >
> > Don't think it's the correct approach.
> >
> > I don't think we ever run g_object_run_dispose()
> ourselves for
> > port
> > objects, and that means that whenever we got the
> port object
> > disposed it
> > was because it was the last valid reference.
> >
> > And that means that you won't be able to read
> priv->disposed
> > afterwards
> > as that would mean reading already freed memory.
> >
> > There clearly is a missing reference around, or a
> timeout or
> > other
> > source which has the port as user_data and is not
> being
> > properly cleaned
> > up (e.g. removing the timeout or event source when
> the port is
> > disposed). Running it under valgrind should confirm
> this. Is
> > this
> > related to the data_available() crashes during
> suspend/resume?
> >
> >
> > >
> > > diff --git a/src/mm-serial-port.c
> b/src/mm-serial-port.c
> > > index 0a8820d..a33c745 100644
> > > --- a/src/mm-serial-port.c
> > > +++ b/src/mm-serial-port.c
> > > @@ -69,6 +69,7 @@ static guint
> signals[LAST_SIGNAL] = { 0 };
> > >
> > > typedef struct {
> > > guint32 open_count;
> > > + gboolean disposed;
> > > gboolean forced_close;
> > > int fd;
> > > GHashTable *reply_cache;
> > > @@ -849,6 +850,12 @@ mm_serial_port_open
> (MMSerialPort
> > *self, GError **error)
> > >
> > > device = mm_port_get_device (MM_PORT (self));
> > >
> > > + /* If the MMSerialPort object has been
> disposed, just
> > return an error. */
> > > + if (priv->disposed) {
> > > + mm_info ("(%s) skipped opening serial
> port that has
> > been disposed", device);
> > > + return FALSE;
> > > + }
> > > +
> > > if (priv->open_count) {
> > > /* Already open */
> > > goto success;
> > > @@ -1537,6 +1544,9 @@ dispose (GObject *object)
> > > {
> > > MMSerialPortPrivate *priv =
> MM_SERIAL_PORT_GET_PRIVATE
> > (object);
> > >
> > > + /* Mark the MMSerialPort object as disposed
> to prevent
> > it from being re-opened. */
> > > + priv->disposed = TRUE;
> > > +
> > > if (priv->timeout_id) {
> > > g_source_remove (priv->timeout_id);
> > > priv->timeout_id = 0;
> > >
> >
> >
> > --
> >
> > Aleksander
> >
> >
>
> > _______________________________________________
> > networkmanager-list mailing list
> > networkmanager-list gnome org
> > https://mail.gnome.org/mailman/listinfo/networkmanager-list
>
>
>
>
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
