[PATCH] serial-port: fail open/reopen after a serial port is disposed

[Ben Chan]

On Wed, Dec 18, 2013 at 6:25 AM, Aleksander Morgado
<aleksander at lanedo.com> wrote:
>
> On 17/12/13 22:48, Ben Chan wrote:
> > After a MMSerialPort object is disposed and closed, it may be opened again.
> > That could potentially lead to a crash if data_available is called after
> > the MMSerialPort object is finalized. This patch prevents such a
> > scenario by failing mm_serial_port_open / mm_serial_port_reopen on an
> > already disposed MMSerialPort object.
> > ---
> > Dan / Aleksander,
> >
> > The code shouldn't really call open/reopen on an already disposed MMSerialPort,
> > I wonder if we should use g_assert (!priv->disposed) instead of reporting an
> > error. That would allow us to pin point the source of the problem more easily.
> > How do you think?
> >
>
> In our current setup we do allow disposed objects which are still not
> yet finished. In other words, we allow to have an object in which
> dispose() was called (e.g. g_object_run_dispose()) and still have valid
> references around (i.e. finalize() wasn't called yet). But, from what I
> can recall, this was only applicable to 'modem' objects, so that we
> could cleanup circular references between objects.
>
> I'm not sure that we allow in any way non-finalized-but-disposed ports,
> unless it's a reference bug (i.e. we're using already unref-ed objects).
>
> After thinking about it further, I believe that the real problem here
> could be that the port got 'forced-close' (modem unplugged and we get
> HUP in the data_available() callback, which triggers forced closing) and
> after that we try to open it again (so the reference is still valid, no
> one called dispose or finalize). When the port disappears we do clean
> the GObjects from the MMBaseModem, *but* there may be some other port
> reference around (e.g. in an async method context we may have a
> reference to the primary port). An easy fix for this would be to return
> an error in open() or reopen() if priv->forced_close is set.
>
> Does this make sense?
>

If some async method or timeout context still holds a reference to the port,
finalize shouldn't have been invoked, right? If finalize is invoked,
that seems like
another reference bug?

Related to the HUP case, is the port always removed by the kernel?
I sometimes observed that ModemManager was able to communicate with the
modem after restarting. so I wonder if the 'port hangup' scenario is temporary.