Date: Mon, 4 Feb 2013 17:08:02 -0500 From: "J.R. Oldroyd" <fbsd@opal.com> To: George Liaskos <geo.liaskos@gmail.com> Cc: freebsd-chromium@freebsd.org Subject: Re: IPC memory leakage on latest chromium-24.0.1312.57 Message-ID: <20130204170802.3ed0ac23@shibato> In-Reply-To: <CANcjpOBFW6xzjtzx36jn_ADSj5zh0dnedXA68=e_HMtr0_EozA@mail.gmail.com> References: <CAJuc1zNLC-ESojGq9B5-_7qr6EiwGEwxUojBE9fkU0GybEr6nw@mail.gmail.com> <1563077977.10385807.1359749865202.JavaMail.root@k-state.edu> <CAF6rxgkaqOw_CwikrWOHC26YRHjx8Ade4KORasZn8WzbmYbrVQ@mail.gmail.com> <20130203122913.6c0cf4e9@shibato> <CANcjpOBFW6xzjtzx36jn_ADSj5zh0dnedXA68=e_HMtr0_EozA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Sig_/D50Q57cyMqMrZiPMK3ZD4P8 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 4 Feb 2013 20:55:58 +0200 George Liaskos <geo.liaskos@gmail.com> wrote: > > > I think the problem may be related to > > files/patch-ui__surface__transport_dib_linux.cc which replaces the > > immediate removal of the shm after attaching to it with removal in > > the destructor iff we're the last who is attached to it. > > > > The comment states: > > // On BSD we can't access the shared memory after is marked for > > deletion. > > but this is not true if kern.ipc.shm_allow_removed=3D1 which we are told > > to set in the pkg-message. >=20 > The problem is definitely in this patch, if kern.ipc.shm_allow_removed > is set then the patch is not needed at all. > The issue here is that the sysctl is not set by default and we must / > should support the default configuration. >=20 > I don't really know how to solve correctly this issue, TransportDIBs > are created from a static function and the shm keys are cached. > Simply removing the key in the destructor is not correct because there > are other TransportDIB instances which use the same key. >=20 > ... and even if you handle the removal gracefully, what happens when > the process crashes? More leaks, that's why all other platforms allow > the removal immediately. >=20 > Maybe we are correct according to the Posix spec but now i understand > why almost no one follows this specific behavior. >=20 >=20 > [1] http://src.chromium.org/viewvc/chrome/trunk/src/ui/surface/transport_= dib_linux.cc?revision=3D167669&content-type=3Dtext%2Fplain Hey George, I saw you were the submitter of this patch, so thanks for chiming in. I can say that I've run without the patch for about 36-48 hours now and the problem has not recurred. So those wanting a simple fix can simply remove the patch, recompile chrome, set kern.ipc.shm_allow_removed=3D1 and run. Since the patch breaks things, I think it needs to be replaced. A simple fix would be to remove the patch and add a wrapper sh script that checks if the sysctl is set and prints a message if not. As for a proper fix, I haven't really studied that code in detail. Some possible thoughts. I am not sure what is using this shm region... separate processes or separate threads? Could the shm perhaps be replaced by something else, such as a malloc'd region accessed through a global variable? If not, is the removal code in the right place in the destructor; should it perhaps be in the TransportDIB::Detach method instead, or somewhere else? Do we know why it is not being executed - is this due to the removal code not being reached or is it due to something crashing? If something is crashing (and we can't fix that problem), can we trap that event and remove the shm there? -jr --Sig_/D50Q57cyMqMrZiPMK3ZD4P8 Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlEQMU0ACgkQls33urr0k4n64wCfa4jL/8deXRrIWRNJZJRFKRgS aw0AoKeR7jrpk6p0+3ez7Yrg2r8AXGXw =ZxoY -----END PGP SIGNATURE----- --Sig_/D50Q57cyMqMrZiPMK3ZD4P8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130204170802.3ed0ac23>