Skip to content
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.

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

CoreCLR initialization failures logging #78790

Merged
merged 7 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
33 changes: 33 additions & 0 deletions src/coreclr/dlls/mscoree/exports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,26 @@ static void ConvertConfigPropertiesToUnicode(
*propertyValuesWRef = propertyValuesW;
}

coreclr_error_writer_callback_fn g_errorWriter = nullptr;

//
// Set callback for writing error logging
//
// Parameters:
// errorWriter - callback that will be called for each line of the error info
// - passing in NULL removes a callback that was previously set
//
Comment on lines +166 to +169
Copy link
Member

Choose a reason for hiding this comment

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

We should probably add that the callback can be called from any thread and thus must be thread safe.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not the case at the moment, it is always called from the thread running the initialization. Do you mean that we should add that comment to make it possible if we ever needed that?

Copy link
Member

Choose a reason for hiding this comment

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

Didn't realize we don't need it currently. But I would add it anyway... it's really easy to forget about this and it could break in really weird ways (the current host implementation is thread safe fortunately).

// Returns:
// S_OK
//
extern "C"
DLLEXPORT
int coreclr_set_error_writer(coreclr_error_writer_callback_fn error_writer)
{
g_errorWriter = error_writer;
return S_OK;
}

#ifdef FEATURE_GDBJIT
GetInfoForMethodDelegate getInfoForMethodDelegate = NULL;
extern "C" int coreclr_create_delegate(void*, unsigned int, const char*, const char*, const char*, void**);
Expand Down Expand Up @@ -432,3 +452,16 @@ int coreclr_execute_assembly(

return hr;
}

void LogErrorToHost(const char* format, ...)
{
if (g_errorWriter != NULL)
{
char messageBuffer[1024];
va_list args;
va_start(args, format);
_vsnprintf_s(messageBuffer, ARRAY_SIZE(messageBuffer), _TRUNCATE, format, args);
g_errorWriter(messageBuffer);
va_end(args);
}
}
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscoree/mscorwks_ntdef.src
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ EXPORTS
coreclr_create_delegate
coreclr_execute_assembly
coreclr_initialize
coreclr_set_error_writer
coreclr_shutdown
coreclr_shutdown_2

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscoree/mscorwks_unixexports.src
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
coreclr_create_delegate
coreclr_execute_assembly
coreclr_initialize
coreclr_set_error_writer
coreclr_shutdown
coreclr_shutdown_2

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/gc/env/gcenv.ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class GCToEEInterface
static uint32_t GetCurrentProcessCpuCount();

static void DiagAddNewRegion(int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved);

static void LogErrorToHost(const char *message);
};

#endif // __GCENV_EE_H__
20 changes: 18 additions & 2 deletions src/coreclr/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13575,13 +13575,17 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size,
gc_log = CreateLogFile(GCConfig::GetLogFile(), false);

if (gc_log == NULL)
{
GCToEEInterface::LogErrorToHost("Cannot create log file");
return E_FAIL;
}

// GCLogFileSize in MBs.
gc_log_file_size = static_cast<size_t>(GCConfig::GetLogFileSize());

if (gc_log_file_size <= 0 || gc_log_file_size > 500)
{
GCToEEInterface::LogErrorToHost("Invalid log file size (valid size needs to be larger than 0 and smaller than 500)");
fclose (gc_log);
return E_FAIL;
}
Expand All @@ -13591,7 +13595,7 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size,
if (!gc_log_buffer)
{
fclose(gc_log);
return E_FAIL;
return E_OUTOFMEMORY;
}

memset (gc_log_buffer, '*', gc_log_buffer_size);
Expand All @@ -13606,13 +13610,16 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size,
gc_config_log = CreateLogFile(GCConfig::GetConfigLogFile(), true);

if (gc_config_log == NULL)
{
GCToEEInterface::LogErrorToHost("Cannot create log file");
return E_FAIL;
}

gc_config_log_buffer = new (nothrow) uint8_t [gc_config_log_buffer_size];
if (!gc_config_log_buffer)
{
fclose(gc_config_log);
return E_FAIL;
return E_OUTOFMEMORY;
}

compact_ratio = static_cast<int>(GCConfig::GetCompactRatio());
Expand Down Expand Up @@ -13739,6 +13746,7 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size,
else
{
assert (!"cannot use regions without specifying the range!!!");
GCToEEInterface::LogErrorToHost("Cannot use regions without specifying the range (using DOTNET_GCRegionRange)");
return E_FAIL;
}
#else //USE_REGIONS
Expand Down Expand Up @@ -13862,6 +13870,7 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size,

if (!init_semi_shared())
{
GCToEEInterface::LogErrorToHost("PER_HEAP_ISOLATED data members initialization failed");
hres = E_FAIL;
}

Expand Down Expand Up @@ -45537,6 +45546,7 @@ HRESULT GCHeap::Initialize()

if (!WaitForGCEvent->CreateManualEventNoThrow(TRUE))
{
GCToEEInterface::LogErrorToHost("Creation of WaitForGCEvent failed");
return E_FAIL;
}

Expand Down Expand Up @@ -45618,9 +45628,15 @@ HRESULT GCHeap::Initialize()
int hb_info_size_per_node = hb_info_size_per_proc * procs_per_numa_node;
uint8_t* numa_mem = (uint8_t*)GCToOSInterface::VirtualReserve (hb_info_size_per_node, 0, 0, numa_node_index);
if (!numa_mem)
{
GCToEEInterface::LogErrorToHost("Reservation of numa_mem failed");
return E_FAIL;
}
if (!GCToOSInterface::VirtualCommit (numa_mem, hb_info_size_per_node, numa_node_index))
{
GCToEEInterface::LogErrorToHost("Commit of numa_mem failed");
return E_FAIL;
}

heap_balance_info_proc* hb_info_procs = (heap_balance_info_proc*)numa_mem;
hb_info_numa_nodes[numa_node_index].hb_info_procs = hb_info_procs;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/gc/gccommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ IGCHeapInternal* g_theGCHeap;
IGCHandleManager* g_theGCHandleManager;

#ifdef BUILD_AS_STANDALONE
IGCToCLR* g_theGCToCLR;
IGCToCLR2* g_theGCToCLR;
VersionInfo g_runtimeSupportedVersion;
#endif // BUILD_AS_STANDALONE

Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/gc/gcenv.ee.standalone.inl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

// The singular interface instance. All calls in GCToEEInterface
// will be forwarded to this interface instance.
extern IGCToCLR* g_theGCToCLR;
extern IGCToCLR2* g_theGCToCLR;

// GC version that the current runtime supports
extern VersionInfo g_runtimeSupportedVersion;
Expand Down Expand Up @@ -314,4 +314,12 @@ inline void GCToEEInterface::DiagAddNewRegion(int generation, uint8_t* rangeStar
g_theGCToCLR->DiagAddNewRegion(generation, rangeStart, rangeEnd, rangeEndReserved);
}

inline void GCToEEInterface::LogErrorToHost(const char *message)
{
if (g_runtimeSupportedVersion.MajorVersion >= GC_INTERFACE2_MAJOR_VERSION)
{
g_theGCToCLR->LogErrorToHost(message);
}
}

#endif // __GCTOENV_EE_STANDALONE_INL__
7 changes: 7 additions & 0 deletions src/coreclr/gc/gcinterface.ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,4 +448,11 @@ class IGCToCLR {
void DiagAddNewRegion(int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved) = 0;
};

class IGCToCLR2 : public IGCToCLR {
public:

virtual
void LogErrorToHost(const char *message) = 0;
};

#endif // _GCINTERFACE_EE_H_
6 changes: 5 additions & 1 deletion src/coreclr/gc/gcinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@

// The major version of the GC/EE interface. Breaking changes to this interface
// require bumps in the major version number.
#define GC_INTERFACE_MAJOR_VERSION 5
#define GC_INTERFACE_MAJOR_VERSION 6

// The minor version of the GC/EE interface. Non-breaking changes are required
// to bump the minor version number. GCs and EEs with minor version number
// mismatches can still interopate correctly, with some care.
#define GC_INTERFACE_MINOR_VERSION 1

// The major version of the GC/EE interface. Breaking changes to this interface
// require bumps in the major version number.
#define GC_INTERFACE2_MAJOR_VERSION 6

struct ScanContext;
struct gc_alloc_context;
class CrawlFrame;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/gc/gcload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ GC_Initialize(

#ifdef BUILD_AS_STANDALONE
assert(clrToGC != nullptr);
g_theGCToCLR = clrToGC;
g_theGCToCLR = (IGCToCLR2*)clrToGC;
#else
UNREFERENCED_PARAMETER(clrToGC);
assert(clrToGC == nullptr);
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/gc/sample/gcenv.ee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,7 @@ uint32_t GCToEEInterface::GetCurrentProcessCpuCount()
void GCToEEInterface::DiagAddNewRegion(int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
{
}

void GCToEEInterface::LogErrorToHost(const char *message)
{
}
4 changes: 4 additions & 0 deletions src/coreclr/gc/unix/gcenv.unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ bool GCToOSInterface::Initialize()
int cpuCount = sysconf(SYSCONF_GET_NUMPROCS);
if (cpuCount == -1)
{
GCToEEInterface::LogErrorToHost("Cannot get CPU count");
return false;
}

Expand All @@ -385,6 +386,7 @@ bool GCToOSInterface::Initialize()

if (g_helperPage == MAP_FAILED)
{
GCToEEInterface::LogErrorToHost("Failed to allocate g_helperPage");
return false;
}

Expand All @@ -398,12 +400,14 @@ bool GCToOSInterface::Initialize()

if (status != 0)
{
GCToEEInterface::LogErrorToHost("Failed to mlock g_helperPage");
return false;
}

status = pthread_mutex_init(&g_flushProcessWriteBuffersMutex, NULL);
if (status != 0)
{
GCToEEInterface::LogErrorToHost("Failed to initialize g_flushProcessWriteBuffersMutex");
munlock(g_helperPage, OS_PAGE_SIZE);
return false;
}
Expand Down
23 changes: 21 additions & 2 deletions src/coreclr/hosts/corerun/corerun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ static string_t build_tpa(const string_t& core_root, const string_t& core_librar
return tpa_list.str();
}

static bool try_get_export(pal::mod_t mod, const char* symbol, void** fptr)
static bool try_get_export(pal::mod_t mod, const char* symbol, void** fptr, bool isOptional = false)
janvorli marked this conversation as resolved.
Show resolved Hide resolved
{
assert(mod != nullptr && symbol != nullptr && fptr != nullptr);
*fptr = pal::get_module_symbol(mod, symbol);
if (*fptr != nullptr)
if ((*fptr != nullptr) || isOptional)
return true;

pal::fprintf(stderr, W("Export '%s' not found.\n"), symbol);
Expand Down Expand Up @@ -205,6 +205,11 @@ class logger_t final
static void* CurrentClrInstance;
static unsigned int CurrentAppDomainId;

static void log_error_info(const char* line)
{
std::fprintf(stderr, "%s\n", line);
}

static int run(const configuration& config)
{
platform_specific_actions actions;
Expand Down Expand Up @@ -282,6 +287,7 @@ static int run(const configuration& config)
// Get CoreCLR exports
coreclr_initialize_ptr coreclr_init_func = nullptr;
coreclr_execute_assembly_ptr coreclr_execute_func = nullptr;
coreclr_set_error_writer_ptr coreclr_set_error_writer_func = nullptr;
coreclr_shutdown_2_ptr coreclr_shutdown2_func = nullptr;
if (!try_get_export(coreclr_mod, "coreclr_initialize", (void**)&coreclr_init_func)
|| !try_get_export(coreclr_mod, "coreclr_execute_assembly", (void**)&coreclr_execute_func)
Expand All @@ -290,6 +296,9 @@ static int run(const configuration& config)
return -1;
}

// The coreclr_set_error_writer is optional
try_get_export(coreclr_mod, "coreclr_set_error_writer", (void**)&coreclr_set_error_writer_func, true /* isOptional */);
janvorli marked this conversation as resolved.
Show resolved Hide resolved

// Construct CoreCLR properties.
pal::string_utf8_t tpa_list_utf8 = pal::convert_to_utf8(tpa_list.c_str());
pal::string_utf8_t app_path_utf8 = pal::convert_to_utf8(app_path.c_str());
Expand Down Expand Up @@ -344,6 +353,11 @@ static int run(const configuration& config)
propertyCount, propertyKeys.data(), propertyValues.data(),
entry_assembly_utf8.c_str(), config.entry_assembly_argc, argv_utf8.get() };

if (coreclr_set_error_writer_func != nullptr)
{
coreclr_set_error_writer_func(log_error_info);
}

int result;
result = coreclr_init_func(
exe_path_utf8.c_str(),
Expand All @@ -361,6 +375,11 @@ static int run(const configuration& config)
return -1;
}

if (coreclr_set_error_writer_func != nullptr)
{
coreclr_set_error_writer_func(nullptr);
}

int exit_code;
{
actions.before_execute_assembly(config.entry_assembly_fullpath);
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/hosts/inc/coreclrhost.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ CORECLR_HOSTING_API(coreclr_initialize,
void** hostHandle,
unsigned int* domainId);

//
// Type of the callback function that can be set by the coreclr_set_error_writer
//
typedef void (*coreclr_error_writer_callback_fn) (const char *message);

//
// Set callback for writing error logging
//
// Parameters:
// errorWriter - callback that will be called for each line of the error info
// - passing in NULL removes a callback that was previously set
//
// Returns:
// S_OK
//
CORECLR_HOSTING_API(coreclr_set_error_writer,
coreclr_error_writer_callback_fn errorWriter);

//
// Shutdown CoreCLR. It unloads the app domain and stops the CoreCLR host.
//
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/nativeaot/Runtime/gcrhenv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,10 @@ bool GCToEEInterface::GetIntConfigValue(const char* privateKey, const char* publ
return true;
}

void GCToEEInterface::LogErrorToHost(const char *message)
{
}

bool GCToEEInterface::GetStringConfigValue(const char* privateKey, const char* publicKey, const char** value)
{
UNREFERENCED_PARAMETER(privateKey);
Expand Down
Loading