Skip to content

Remove unused members on runtime Thread #105158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jkotas merged 4 commits into from
Jul 23, 2024
Merged

Remove unused members on runtime Thread #105158

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1eb07c0
Remove unused members on runtime Thread
elinor-fung Jul 19, 2024
173981e
Use SOleTlsData definition from public doc
elinor-fung Jul 22, 2024
c440103
Remove forward declaration
elinor-fung Jul 22, 2024
8430348
Update src/coreclr/vm/oletls.h
elinor-fung Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/coreclr/inc/contract.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1981,21 +1981,6 @@ inline ClrDebugState *GetClrDebugState(BOOL fAlloc)
#define LOCK_RELEASED_MULTIPLE(dbgStateLockType, cExits, pvLock) \
::GetClrDebugState()->LockReleased((dbgStateLockType), (cExits), (void*) (pvLock))

// Use these only if you need to force multiple entrances or exits in a single
// line (e.g., to restore the lock to a previous state). CRWLock in vm\rwlock.cpp does this
#define EE_LOCK_TAKEN_MULTIPLE(cEntrances, pvLock) \
LOCK_TAKEN_MULTIPLE(kDbgStateLockType_EE, cEntrances, pvLock)
#define EE_LOCK_RELEASED_MULTIPLE(cExits, pvLock) \
LOCK_RELEASED_MULTIPLE(kDbgStateLockType_EE, cExits, pvLock)
#define HOST_BREAKABLE_CRST_TAKEN_MULTIPLE(cEntrances, pvLock) \
LOCK_TAKEN_MULTIPLE(kDbgStateLockType_HostBreakableCrst, cEntrances, pvLock)
#define HOST_BREAKABLE_CRST_RELEASED_MULTIPLE(cExits, pvLock) \
LOCK_RELEASED_MULTIPLE(kDbgStateLockType_HostBreakableCrst, cExits, pvLock)
#define USER_LOCK_TAKEN_MULTIPLE(cEntrances, pvLock) \
LOCK_TAKEN_MULTIPLE(kDbgStateLockType_User, cEntrances, pvLock)
#define USER_LOCK_RELEASED_MULTIPLE(cExits, pvLock) \
LOCK_RELEASED_MULTIPLE(kDbgStateLockType_User, cExits, pvLock)

// These are most typically used
#define EE_LOCK_TAKEN(pvLock) \
LOCK_TAKEN_MULTIPLE(kDbgStateLockType_EE, 1, pvLock)
Expand All @@ -2014,12 +1999,6 @@ inline ClrDebugState *GetClrDebugState(BOOL fAlloc)

#define LOCK_TAKEN_MULTIPLE(dbgStateLockType, cEntrances, pvLock)
#define LOCK_RELEASED_MULTIPLE(dbgStateLockType, cExits, pvLock)
#define EE_LOCK_TAKEN_MULTIPLE(cEntrances, pvLock)
#define EE_LOCK_RELEASED_MULTIPLE(cExits, pvLock)
#define HOST_BREAKABLE_CRST_TAKEN_MULTIPLE(cEntrances, pvLock)
#define HOST_BREAKABLE_CRST_RELEASED_MULTIPLE(cExits, pvLock)
#define USER_LOCK_TAKEN_MULTIPLE(cEntrances, pvLock)
#define USER_LOCK_RELEASED_MULTIPLE(cExits, pvLock)
#define EE_LOCK_TAKEN(pvLock)
#define EE_LOCK_RELEASED(pvLock)
#define HOST_BREAKABLE_CRST_TAKEN(pvLock)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/olecontexthelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ LPVOID SetupOleContext()
SOleTlsData* _pData = (SOleTlsData *) ClrTeb::GetOleReservedPtr();
if (_pData && _pData->pCurrentCtx == NULL)
{
_pData->pCurrentCtx = (CObjectContext*)pObjCtx; // no release !!!!
_pData->pCurrentCtx = pObjCtx; // no release !!!!
}
else
{
Expand Down
207 changes: 10 additions & 197 deletions src/coreclr/vm/oletls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,5 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//+---------------------------------------------------------------------------
//
// File: oletls.h
//

//
// Purpose: manage thread local storage for OLE
//
// Notes: The gTlsIndex is initialized at process attach time.
// The per-thread data is allocated in CoInitialize in
// single-threaded apartments or on first use in
// multi-threaded apartments.
//
//----------------------------------------------------------------------------

#ifndef _OLETLS_H_
#define _OLETLS_H_
Expand All @@ -22,188 +8,15 @@
#error FEATURE_COMINTEROP_APARTMENT_SUPPORT is required for this file
#endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT

//+---------------------------------------------------------------------------
//
// forward declarations (in order to avoid type casting when accessing
// data members of the SOleTlsData structure).
//
//+---------------------------------------------------------------------------

class CAptCallCtrl; // see callctrl.hxx
class CSrvCallState; // see callctrl.hxx
class CObjServer; // see sobjact.hxx
class CSmAllocator; // see stg\h\smalloc.hxx
class CMessageCall; // see call.hxx
class CClientCall; // see call.hxx
class CAsyncCall; // see call.hxx
class CClipDataObject; // see ole232\clipbrd\clipdata.h
class CSurrogatedObjectList; // see com\inc\comsrgt.hxx
class CCtxCall; // see PSTable.hxx
class CPolicySet; // see PSTable.hxx
class CObjectContext; // see context.hxx
class CComApartment; // see aprtmnt.hxx

//+-------------------------------------------------------------------
//
// Struct: CallEntry
//
// Synopsis: Call Table Entry.
//
//+-------------------------------------------------------------------
typedef struct tagCallEntry
{
void *pNext; // ptr to next entry
void *pvObject; // Entry object
} CallEntry;



//+---------------------------------------------------------------------------
//
// Enum: OLETLSFLAGS
//
// Synopsys: bit values for dwFlags field of SOleTlsData. If you just want
// to store a BOOL in TLS, use this enum and the dwFlag field.
//
//+---------------------------------------------------------------------------
typedef enum tagOLETLSFLAGS
{
OLETLS_LOCALTID = 0x01, // This TID is in the current process.
OLETLS_UUIDINITIALIZED = 0x02, // This Logical thread is init'd.
OLETLS_INTHREADDETACH = 0x04, // This is in thread detach. Needed
// due to NT's special thread detach
// rules.
OLETLS_CHANNELTHREADINITIALZED = 0x08,// This channel has been init'd
OLETLS_WOWTHREAD = 0x10, // This thread is a 16-bit WOW thread.
OLETLS_THREADUNINITIALIZING = 0x20, // This thread is in CoUninitialize.
OLETLS_DISABLE_OLE1DDE = 0x40, // This thread can't use a DDE window.
OLETLS_APARTMENTTHREADED = 0x80, // This is an STA apartment thread
OLETLS_MULTITHREADED = 0x100, // This is an MTA apartment thread
OLETLS_IMPERSONATING = 0x200, // This thread is impersonating
OLETLS_DISABLE_EVENTLOGGER = 0x400, // Prevent recursion in event logger
OLETLS_INNEUTRALAPT = 0x800, // This thread is in the NTA
OLETLS_DISPATCHTHREAD = 0x1000, // This is a dispatch thread
OLETLS_HOSTTHREAD = 0x2000, // This is a host thread
OLETLS_ALLOWCOINIT = 0x4000, // This thread allows inits
OLETLS_PENDINGUNINIT = 0x8000, // This thread has pending uninit
OLETLS_FIRSTMTAINIT = 0x10000,// First thread to attempt an MTA init
OLETLS_FIRSTNTAINIT = 0x20000,// First thread to attempt an NTA init
OLETLS_APTINITIALIZING = 0x40000 // Apartment Object is initializing
} OLETLSFLAGS;


//+---------------------------------------------------------------------------
//
// Structure: SOleTlsData
//
// Synopsis: structure holding per thread state needed by OLE32
//
//+---------------------------------------------------------------------------
typedef struct tagSOleTlsData
{
#if !defined(_CHICAGO_)
// Docfile multiple allocator support
void *pvThreadBase; // per thread base pointer
CSmAllocator *pSmAllocator; // per thread docfile allocator
#endif
DWORD dwApartmentID; // Per thread "process ID"
DWORD dwFlags; // see OLETLSFLAGS above

LONG TlsMapIndex; // index in the global TLSMap
void **ppTlsSlot; // Back pointer to the thread tls slot
DWORD cComInits; // number of per-thread inits
DWORD cOleInits; // number of per-thread OLE inits

DWORD cCalls; // number of outstanding calls
CMessageCall *pCallInfo; // channel call info
CAsyncCall *pFreeAsyncCall; // ptr to available call object for this thread.
CClientCall *pFreeClientCall; // ptr to available call object for this thread.

CObjServer *pObjServer; // Activation Server Object for this apartment.
DWORD dwTIDCaller; // TID of current calling app
CObjectContext *pCurrentCtx; // Current context
CObjectContext *pEmptyCtx; // Empty context

CObjectContext *pNativeCtx; // Native context
CComApartment *pNativeApt; // Native apartment for the thread.
IUnknown *pCallContext; // call context object
CCtxCall *pCtxCall; // Context call object

CPolicySet *pPS; // Policy set
PVOID pvPendingCallsFront;// Per Apt pending async calls
PVOID pvPendingCallsBack;
CAptCallCtrl *pCallCtrl; // call control for RPC for this apartment

CSrvCallState *pTopSCS; // top server-side callctrl state
IMessageFilter *pMsgFilter; // temp storage for App MsgFilter
HWND hwndSTA; // STA server window same as poxid->hServerSTA
// ...needed on Win95 before oxid registration
LONG cORPCNestingLevel; // call nesting level (DBG only)

DWORD cDebugData; // count of bytes of debug data in call
ULONG cPreRegOidsAvail; // count of server-side OIDs avail
unsigned hyper *pPreRegOids; // ptr to array of pre-reg OIDs

UUID LogicalThreadId; // current logical thread id

HANDLE hThread; // Thread handle used for cancel
HANDLE hRevert; // Token before first impersonate.
IUnknown *pAsyncRelease; // Controlling unknown for async release
// DDE data
HWND hwndDdeServer; // Per thread Common DDE server

HWND hwndDdeClient; // Per thread Common DDE client
ULONG cServeDdeObjects; // non-zero if objects DDE should serve
// ClassCache data
LPVOID pSTALSvrsFront; // Chain of LServers registers in this thread if STA
// upper layer data
HWND hwndClip; // Clipboard window

IDataObject *pDataObjClip; // Current Clipboard DataObject
DWORD dwClipSeqNum; // Clipboard Sequence # for the above DataObject
DWORD fIsClipWrapper; // Did we hand out the wrapper Clipboard DataObject?
IUnknown *punkState; // Per thread "state" object
// cancel data
DWORD cCallCancellation; // count of CoEnableCallCancellation
// async sends data
DWORD cAsyncSends; // count of async sends outstanding

CAsyncCall* pAsyncCallList; // async calls outstanding
CSurrogatedObjectList *pSurrogateList; // Objects in the surrogate

LockEntry lockEntry; // Locks currently held by the thread
CallEntry CallEntry; // client-side call chain for this thread

#ifdef WX86OLE
IUnknown *punkStateWx86; // Per thread "state" object for Wx86
#endif
void *pDragCursors; // Per thread drag cursor table.

#ifdef _CHICAGO_
LPVOID pWcstokContext; // Scan context for wcstok
#endif

IUnknown *punkError; // Per thread error object.
ULONG cbErrorData; // Maximum size of error data.

#if(_WIN32_WINNT >= 0x0500)
IUnknown *punkActiveXSafetyProvider;
#endif //(_WIN32_WINNT >= 0x0500)

#if DBG==1
LONG cTraceNestingLevel; // call nesting level for OLETRACE
#endif

} SOleTlsData;

#ifdef INITGUID
#include "initguid.h"
#endif

#define DEFINE_OLEGUID(name, l, w1, w2) \
DEFINE_GUID(name, l, w1, w2, 0xC0,0,0,0,0,0,0,0x46)

DEFINE_OLEGUID(IID_IStdIdentity, 0x0000001bL, 0, 0);
DEFINE_OLEGUID(IID_IStdWrapper, 0x000001caL, 0, 0);
// See https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/ms690269(v=vs.85)
Comment thread
elinor-fung marked this conversation as resolved.
Outdated
typedef struct _SOleTlsData {
void *pvReserved0[2];
DWORD dwReserved0[3];
void *pvReserved1[1];
DWORD dwReserved1[3];
void *pvReserved2[4];
DWORD dwReserved2[1];
void *pCurrentCtx;
} SOleTlsData, *PSOleTlsData;

#endif // _OLETLS_H_
8 changes: 0 additions & 8 deletions src/coreclr/vm/threads.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1330,14 +1330,6 @@ Thread::Thread()

m_dwForbidSuspendThread = 0;

// Initialize lock state
m_pHead = &m_embeddedEntry;
m_embeddedEntry.pNext = m_pHead;
m_embeddedEntry.pPrev = m_pHead;
m_embeddedEntry.dwLLockID = 0;
m_embeddedEntry.dwULockID = 0;
m_embeddedEntry.wReaderLevel = 0;

m_pBlockingLock = NULL;

m_pRuntimeThreadLocals = nullptr;
Expand Down
17 changes: 0 additions & 17 deletions src/coreclr/vm/threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ class EECodeInfo;
class DebuggerPatchSkip;
class FaultingExceptionFrame;
enum BinderMethodID : int;
class CRWLock;
struct LockEntry;
class PrepareCodeConfig;
class NativeCodeVersion;

Expand Down Expand Up @@ -379,16 +377,6 @@ EXTERN_C void ThrowControlForThread(
#endif // FEATURE_EH_FUNCLETS
);

// RWLock state inside TLS
struct LockEntry
{
LockEntry *pNext; // next entry
LockEntry *pPrev; // prev entry
LONG dwULockID;
LONG dwLLockID; // owning lock
WORD wReaderLevel; // reader nesting level
};

#if defined(_DEBUG)
BOOL MatchThreadHandleToOsId ( HANDLE h, DWORD osId );
#endif
Expand Down Expand Up @@ -952,11 +940,6 @@ class Thread
// in the object header to store it.
DWORD m_ThreadId;


// RWLock state
LockEntry *m_pHead;

@jkotas jkotas Jul 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we delete LockEntry completely, or at least more it to oletls.h that is its only use?

We do not need the full definition of SOleTlsData. We only need the part up until the fields that we depend on. The rest can be deleted. (A separate question is whether it is a good idea to depend on SOleTlsData that is Windows internal impl detail.)

@jkoritzinsky jkoritzinsky Jul 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the only portion of SOleTlsData that we use is the context pointer. Would it be possible for us to use CoGetObjectContext directly instead of going through the SOleTlsData object? Then we could remove it entirely.

@jkotas jkotas Jul 20, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would take some perf regression by replacing the shortcut with CoGetObjectContext. Also, we set the SOleTlsData field here:

runtime/src/coreclr/vm/olecontexthelpers.cpp

Line 54 in 043ae8c

_pData->pCurrentCtx = (CObjectContext*)pObjCtx; // no release !!!!
... I am not sure what to do about that.

@jkoritzinsky jkoritzinsky Jul 20, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I saw that. I wonder if we ever hit that path. My guess would be there was some bug in Windows that didn't set that sometimes so .NET Framework would do it. I really hope that path doesn't actually get hit in production...

At least since that's the only field we use, we could use the official definition of the struct from the docs for a slightly better experience.

Also, could we implement the fast path with our own thread-local variable instead of using this implementation detail?

@jkotas jkotas Jul 20, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least since that's the only field we use, we could use the official definition of the struct from the docs for a slightly better experience.

Yeah, I think it would make sense to do it in this PR. It should be a simple change.

I agree that it would be nice to clean it up further, but I am not sure whether it is worth spending time on it. It is guaranteed to break something obscure.

LockEntry m_embeddedEntry;

#ifndef DACCESS_COMPILE
Frame* NotifyFrameChainOfExceptionUnwind(Frame* pStartFrame, LPVOID pvLimitSP);
#endif // DACCESS_COMPILE
Expand Down