Modify

Ticket #4885 (closed Bugs: fixed)

Opened 3 years ago

Last modified 22 months ago

Access violation in set_tss_data at process exit due to invalid assumption about TlsAlloc

Reported by: cnewbold Owned by: viboes
Milestone: Boost 1.51.0 Component: thread
Version: Boost 1.44.0 Severity: Showstopper
Keywords: tss Cc: jonathan.jones@…

Description

We've recently upgraded to Boost 1.44 and have started seeing Access Violations from set_tss_data during process exit under various conditions. We are building with Visual Studio 2008 and are seeing the problems on both 32- and 64-bit architectures.

Here's an example stack trace from a crash:

	boost_thread-vc90-mt-1_44.dll!boost::detail::heap_new_impl<boost::detail::tss_data_node,void const * __ptr64 & __ptr64,boost::shared_ptr<boost::detail::tss_cleanup_function> & __ptr64,void * __ptr64 & __ptr64,boost::detail::tss_data_node * __ptr64 & __ptr64>(const void * & a1=, boost::shared_ptr<boost::detail::tss_cleanup_function> & a2={...}, void * & a3=0x00000000003a6c40, boost::detail::tss_data_node * & a4=0x9b0d8d481675c085)  Line 208 + 0x20 bytes	C++
 	boost_thread-vc90-mt-1_44.dll!boost::detail::set_tss_data(const void * key=0x000000005d009600, boost::shared_ptr<boost::detail::tss_cleanup_function> * func=0x00000000001efc28, void * tss_data=0x0000000000000000, bool cleanup_existing=true)  Line 590	C++
 	libut.dll!`anonymous namespace'::`dynamic atexit destructor for 'ticTocPrevTotalsVector''()  + 0x38 bytes	C++
>	libut.dll!_CRT_INIT(void * hDllHandle=0x0000000000000001, unsigned long dwReason=0, void * lpreserved=0x0000000000000000)  Line 449	C
 	libut.dll!__DllMainCRTStartup(void * hDllHandle=0x000000000038f180, unsigned long dwReason=3757760, void * lpreserved=0x000000005cfa6b48)  Line 560 + 0xd bytes	C
 	ntdll.dll!0000000077b33801() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 	ntdll.dll!0000000077b33610() 	
 	msvcr90.dll!00000000660a1b8b() 	
 	test_manager.dll!runTests(int argc=1, char * * argv=0x00000000006a6890)  Line 768 + 0x8 bytes	C++
 	pkgtest.exe!main(int argc=0, char * * argv=0x0000024d06b13a83)  Line 14 + 0x59 bytes	C++
 	pkgtest.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	kernel32.dll!0000000077a0f56d() 	
 	ntdll.dll!0000000077b43281() 	

After some digging, it appears that there is an invalid assumption about TlsAlloc? in thread/src/win32/thread.cpp: namely that it cannot return zero. The tss implementation uses zero as a sentinel value for initialization. As far as I can tell, however, the only "illegal" return value for TlsAlloc? is the constant TLS_OUT_OF_INDEXES, which is defined as -1.

It appears that TlsAlloc? happily returns zero as a valid index when called during process shutdown.

I looked at the solution for #4736 which is on the trunk, but it appears to make the same assumption that TlsAlloc? cannot return zero.

Attachments

4885.patch Download (2.1 KB) - added by viboes 2 years ago.
win32/pthread.cpp
boost thread ticket4885.patch Download (2.4 KB) - added by Ulrich Eckhardt <ulrich.eckhardt@…> 2 years ago.
patch

Change History

comment:1 Changed 3 years ago by cnewbold

  • Owner set to anthonyw
  • Component changed from None to thread

comment:2 Changed 3 years ago by martin.ankerl@…

I have got the same problem with boost 1.43 in combination with boost log. Is there any workaround possible for this?

comment:3 Changed 2 years ago by viboes

  • Keywords tss added

comment:4 Changed 2 years ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from new to assigned
  • Milestone changed from To Be Determined to Boost 1.49.0

After reading the TlsAlloc? doc it seems clear that there is a misunderstanding on the code. I will try to fix it soon.

Changed 2 years ago by viboes

win32/pthread.cpp

comment:5 Changed 2 years ago by viboes

Please, could someone try this patch? Could you attach an example that shows the issue?

comment:6 Changed 2 years ago by anonymous

Committed in trunk at revision: 76752

comment:7 Changed 2 years ago by Ulrich Eckhardt <ulrich.eckhardt@…>

In r76752, tls_out_of_index was actually declared as a variable, even though that is actually a constant. Otherwise, the changes there are completely right. I'll attach a patch, hopefully making it clear what I mean and also with a better workaround for the according constant missing in some CE SDKs.

Changed 2 years ago by Ulrich Eckhardt <ulrich.eckhardt@…>

patch

comment:8 Changed 2 years ago by viboes

Thanks for the patch. I will merge it soon.

comment:9 Changed 2 years ago by viboes

  • Milestone changed from Boost 1.49.0 to Boost 1.51.0

comment:10 Changed 2 years ago by viboes

  • Milestone changed from Boost 1.51.0 to To Be Determined

comment:11 Changed 22 months ago by Jonathan Jones <jonathan.jones@…>

  • Cc jonathan.jones@… added

comment:12 Changed 22 months ago by viboes

Seems to be fixed with #7066

comment:13 Changed 22 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.51.0

comment:14 Changed 22 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

Committed revision [79373].

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.