[PATCHES][BUG 13170] Fix crash in pthreadpool thread after failure from pthread_create

Andreas Schneider asn at samba.org
Wed Dec 6 20:38:02 UTC 2017


On Wednesday, 6 December 2017 21:30:23 CET Christof Schmitt wrote:
> On Wed, Dec 06, 2017 at 06:19:04PM +0100, Volker Lendecke wrote:
> > On Tue, Dec 05, 2017 at 04:23:18PM -0700, Christof Schmitt via samba-
technical wrote:
> > > Another small update. I forgot to clear the inject variable at the end
> > > of the test. That is not relevant for the test added here, but it causes
> > > problems when adding more tests after that.
> > 
> > Patches look good. RB+ on the first two ones. Regarding the test: Yes,
> > it's nasty, but do all linkers support the --wrap option? Do we have
> > to #ifdef this depending on a configure check? If we can't get a
> > resolution on this in soon, I'd vote to put in the bugfix without the
> > test with the promise to actually work on the test. I know, famous
> > last words, but this is important enough to not be held back by a
> > cmocka vs linker flag discussion.
> 
> Andreas, please correct me in case i am missing something here:
> 
> My understanding is that intercepting the function calls in cmocka
> relies on the linker wrap. That is pointed out in the lwn article and
> can also be seen in the example provided by cmocka:
> 
> https://git.cryptomilk.org/projects/cmocka.git/tree/example/chef_wrap/CMakeL
> ists.txt#n16

Yes, that's correct.

> The only addition that cmocka then provides is a global data structure
> that can be used to pass information from the test to the wrap function.

Correct.

> That data structure is only initialized when a cmocka test fixture is
> used. Given that the linker wrap applies to the whole file, this would
> require converting all tests to cmocka.

If you want to convert all tests ...

> To allow backports, i would suggest to use the approach i submitted in
> my patches. That easily applies back to 4.6 where no cmocka is in the
> source tree. cmocka support could be added later, independent of the bug
> fix here.
> 
> If we decide to go with cmocka anyway, the best approach would be to put
> the new test in a new file and implement it completely with cmocka.
> Existing tests could then be converted later.

I thought that we would go that way for master. Just write that one test using 
cmocka. We could do the backport without the test for Samba 4.6, Samba 4.7 
includes cmocka in third_party.

If we want to use cmocka for every test we need to provide a way for mutexes 
without linking cmocka to a threading library. cmocka only requires libc and 
it should stay that way. However we could create it in such a way that you can 
set functions for locking which called at the right positions in cmocka.


Cheers,


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org



More information about the samba-technical mailing list