Date: Fri, 31 Aug 2012 16:37:34 +0300 From: Andrey Simonenko <simon@comsys.ntu-kpi.kiev.ua> To: John Baldwin <jhb@freebsd.org> Cc: Garrett Cooper <yanegomi@gmail.com>, freebsd-current@freebsd.org, Pedro Giffuni <pfg@freebsd.org> Subject: Re: [CFT] Some updates to libc/rpc (second try) Message-ID: <20120831133734.GA45044@pm513-1.comsys.ntu-kpi.kiev.ua> In-Reply-To: <201208310812.09902.jhb@freebsd.org> References: <503ED6A1.1060902@FreeBSD.org> <CAGH67wTTw-waCMcgNoxCD3Dz4Bevka6yW5TG73_U_pd0pqTB-w@mail.gmail.com> <20120831110653.GA21089@pm513-1.comsys.ntu-kpi.kiev.ua> <201208310812.09902.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 31, 2012 at 08:12:09AM -0400, John Baldwin wrote: > On Friday, August 31, 2012 7:06:53 am Andrey Simonenko wrote: > > On Thu, Aug 30, 2012 at 02:37:17AM -0700, Garrett Cooper wrote: > > > > Detailed description of mistakes in these files and correct > implementation: > > > > > > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165710 > > > > > > A developer at $work (Isilon) developed a slightly simpler patch than > > > that based on our custom 7.x sources recently to deal with concurrency > > > issues in netconfig. I'll talk with a couple people to see whether or > > > not the solution can be contributed back [after some polishing -- > > > maybe -- and further testing]. > > > > Can you post changes and corrected bugs in getnetconfig.c and getnetpath.c > > in your (your $work) implementation. > > There is a thread on threads@ with patches to make the API thread-safe > that I believe came from Isilon. It was posted in the last week or so, > so it should be easy to find in the archives. > Thank you for this information. I checked the logic of their changes. Making all data per-thread is wrong from my point of view. 1. Several threads can call getnetconfig() using the same handler obtained by setnetconfig(). If each thread has own FILE pointer, then some getnetconfig() will crash a program since fgets() will be called for NULL FILE pointer. 2. One thread can get handler by setnetconfig() and pass this handler to another thread and getnetconfig() will crash a program. This is the similar mistake, but getnetconfig() is not called in parallel. 3. Each thread has to open netconfig(5) database, parse its content every time for each getnetconfig() call since data is not cached. This is slow. There is one per-thread value, it is error code of the last failed function, this value cannot be kept in handler, since nc_perror() and nc_sperror() do not have handler argument. Since printing error is expected in the same thread when getnetconfig() failed (for example), I think this is Ok. If error is print in another thread, then we simply will not see correct error message, but program will not be crashed. I just commented only per-thread idea of data in getnetconfig.c, I do not compare my implementation, since their patch does not correct any other mistake described and corrected in my PR.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120831133734.GA45044>