ICU-13712 ICU4C does not report OOM if it fails to memory map the data file(s) (#30)

ICU does not report Out-Of-Memory (OOM) if it fails to memory map the data file(s) when calling the various platform API(s) to do so.

When you are using ICU with memory-mapped data file(s), and ICU fails to map the data file due to being out-of-memory, it does not bubble this failure up to the API that was called. You will instead get back the error U_MISSING_RESOURCE_ERROR, rather than U_MEMORY_ALLOCATION_ERROR, which might be a bit surprising to the caller of the API. This can lead to the application thinking that there are no resources for "en_US" or "en" (or even "root").

This change modifies ICU4C so that it will report back U_MEMORY_ALLOCATION_ERROR if OOM happens when attempting to load the data files.
This commit is contained in:
Jeff Genovy 2018-08-29 18:50:50 -07:00 committed by Shane Carr
parent 82cad0d03c
commit eae698a61e
No known key found for this signature in database
GPG Key ID: FCED3B24AAB18B5C
4 changed files with 100 additions and 28 deletions

View File

@ -759,16 +759,19 @@ openCommonData(const char *path, /* Path from OpenChoice? */
UDataPathIterator iter(u_getDataDirectory(), inBasename, path, ".dat", TRUE, pErrorCode);
while((UDataMemory_isLoaded(&tData)==FALSE) && (pathBuffer = iter.next(pErrorCode)) != NULL)
while ((UDataMemory_isLoaded(&tData)==FALSE) && (pathBuffer = iter.next(pErrorCode)) != NULL)
{
#ifdef UDATA_DEBUG
fprintf(stderr, "ocd: trying path %s - ", pathBuffer);
#endif
uprv_mapFile(&tData, pathBuffer);
uprv_mapFile(&tData, pathBuffer, pErrorCode);
#ifdef UDATA_DEBUG
fprintf(stderr, "%s\n", UDataMemory_isLoaded(&tData)?"LOADED":"not loaded");
#endif
}
if (U_FAILURE(*pErrorCode)) {
return NULL;
}
#if defined(OS390_STUBDATA) && defined(OS390BATCH)
if (!UDataMemory_isLoaded(&tData)) {
@ -777,7 +780,7 @@ openCommonData(const char *path, /* Path from OpenChoice? */
uprv_strncpy(ourPathBuffer, path, 1019);
ourPathBuffer[1019]=0;
uprv_strcat(ourPathBuffer, ".dat");
uprv_mapFile(&tData, ourPathBuffer);
uprv_mapFile(&tData, ourPathBuffer, pErrorCode);
}
#endif
@ -868,7 +871,7 @@ static UBool extendICUData(UErrorCode *pErr)
umtx_unlock(&extendICUDataMutex);
#endif
return didUpdate; /* Return true if ICUData pointer was updated. */
/* (Could potentialy have been done by another thread racing */
/* (Could potentially have been done by another thread racing */
/* us through here, but that's fine, we still return true */
/* so that current thread will also examine extended data. */
}
@ -994,12 +997,12 @@ static UDataMemory *doLoadFromIndividualFiles(const char *pkgName,
/* init path iterator for individual files */
UDataPathIterator iter(dataPath, pkgName, path, tocEntryPathSuffix, FALSE, pErrorCode);
while((pathBuffer = iter.next(pErrorCode)) != NULL)
while ((pathBuffer = iter.next(pErrorCode)) != NULL)
{
#ifdef UDATA_DEBUG
fprintf(stderr, "UDATA: trying individual file %s\n", pathBuffer);
#endif
if(uprv_mapFile(&dataMemory, pathBuffer))
if (uprv_mapFile(&dataMemory, pathBuffer, pErrorCode))
{
pEntryData = checkDataItem(dataMemory.pHeader, isAcceptable, context, type, name, subErrorCode, pErrorCode);
if (pEntryData != NULL) {
@ -1015,7 +1018,7 @@ static UDataMemory *doLoadFromIndividualFiles(const char *pkgName,
return pEntryData;
}
/* the data is not acceptable, or some error occured. Either way, unmap the memory */
/* the data is not acceptable, or some error occurred. Either way, unmap the memory */
udata_close(&dataMemory);
/* If we had a nasty error, bail out completely. */
@ -1084,6 +1087,11 @@ static UDataMemory *doLoadFromCommonData(UBool isICUData, const char * /*pkgName
}
}
}
// If we failed due to being out-of-memory, then stop early and report the error.
if (*subErrorCode == U_MEMORY_ALLOCATION_ERROR) {
*pErrorCode = *subErrorCode;
return NULL;
}
/* Data wasn't found. If we were looking for an ICUData item and there is
* more data available, load it and try again,
* otherwise break out of this loop. */

View File

@ -64,7 +64,7 @@
# include "unicode/udata.h"
# define LIB_PREFIX "lib"
# define LIB_SUFFIX ".dll"
/* This is inconvienient until we figure out what to do with U_ICUDATA_NAME in utypes.h */
/* This is inconvenient until we figure out what to do with U_ICUDATA_NAME in utypes.h */
# define U_ICUDATA_ENTRY_NAME "icudt" U_ICU_VERSION_SHORT U_LIB_SUFFIX_C_NAME_STRING "_dat"
# endif
#elif MAP_IMPLEMENTATION==MAP_STDIO
@ -84,7 +84,10 @@
*----------------------------------------------------------------------------*/
#if MAP_IMPLEMENTATION==MAP_NONE
U_CFUNC UBool
uprv_mapFile(UDataMemory *pData, const char *path) {
uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) {
if (U_FAILURE(*status)) {
return FALSE;
}
UDataMemory_init(pData); /* Clear the output struct. */
return FALSE; /* no file access */
}
@ -97,12 +100,17 @@
uprv_mapFile(
UDataMemory *pData, /* Fill in with info on the result doing the mapping. */
/* Output only; any original contents are cleared. */
const char *path /* File path to be opened/mapped */
const char *path, /* File path to be opened/mapped. */
UErrorCode *status /* Error status, used to report out-of-memory errors. */
)
{
HANDLE map;
HANDLE file;
if (U_FAILURE(*status)) {
return FALSE;
}
UDataMemory_init(pData); /* Clear the output struct. */
/* open the input file */
@ -132,7 +140,12 @@
// TODO: Is it worth setting extended parameters to specify random access?
file = CreateFile2(utf16Path, GENERIC_READ, FILE_SHARE_READ, OPEN_EXISTING, NULL);
#endif
if(file==INVALID_HANDLE_VALUE) {
if (file == INVALID_HANDLE_VALUE) {
// If we failed to open the file due to an out-of-memory error, then we want
// to report that error back to the caller.
if (HRESULT_FROM_WIN32(GetLastError()) == E_OUTOFMEMORY) {
*status = U_MEMORY_ALLOCATION_ERROR;
}
return FALSE;
}
@ -165,7 +178,12 @@
map = CreateFileMappingFromApp(file, NULL, PAGE_READONLY, 0, NULL);
#endif
CloseHandle(file);
if(map==NULL) {
if (map == NULL) {
// If we failed to create the mapping due to an out-of-memory error, then
// we want to report that error back to the caller.
if (HRESULT_FROM_WIN32(GetLastError()) == E_OUTOFMEMORY) {
*status = U_MEMORY_ALLOCATION_ERROR;
}
return FALSE;
}
@ -193,12 +211,16 @@
#elif MAP_IMPLEMENTATION==MAP_POSIX
U_CFUNC UBool
uprv_mapFile(UDataMemory *pData, const char *path) {
uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) {
int fd;
int length;
struct stat mystat;
void *data;
if (U_FAILURE(*status)) {
return FALSE;
}
UDataMemory_init(pData); /* Clear the output struct. */
/* determine the length of the file */
@ -221,6 +243,7 @@
#endif
close(fd); /* no longer needed */
if(data==MAP_FAILED) {
// Possibly check the errno value for ENOMEM, and report U_MEMORY_ALLOCATION_ERROR?
return FALSE;
}
@ -263,11 +286,15 @@
}
U_CFUNC UBool
uprv_mapFile(UDataMemory *pData, const char *path) {
uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) {
FILE *file;
int32_t fileLength;
void *p;
if (U_FAILURE(*status)) {
return FALSE;
}
UDataMemory_init(pData); /* Clear the output struct. */
/* open the input file */
file=fopen(path, "rb");
@ -286,6 +313,7 @@
p=uprv_malloc(fileLength);
if(p==NULL) {
fclose(file);
*status = U_MEMORY_ALLOCATION_ERROR;
return FALSE;
}
@ -351,7 +379,7 @@
*
* TODO: This works the way ICU historically has, but the
* whole data fallback search path is so complicated that
* proabably almost no one will ever really understand it,
* probably almost no one will ever really understand it,
* the potential for confusion is large. (It's not just
* this one function, but the whole scheme.)
*
@ -391,7 +419,7 @@
# define DATA_TYPE "dat"
U_CFUNC UBool uprv_mapFile(UDataMemory *pData, const char *path) {
U_CFUNC UBool uprv_mapFile(UDataMemory *pData, const char *path, UErrorCode *status) {
const char *inBasename;
char *basename;
char pathBuffer[1024];
@ -399,6 +427,10 @@
dllhandle *handle;
void *val=0;
if (U_FAILURE(*status)) {
return FALSE;
}
inBasename=uprv_strrchr(path, U_FILE_SEP_CHAR);
if(inBasename==NULL) {
inBasename = path;
@ -430,6 +462,7 @@
data=mmap(0, length, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd); /* no longer needed */
if(data==MAP_FAILED) {
// Possibly check the errorno value for ENOMEM, and report U_MEMORY_ALLOCATION_ERROR?
return FALSE;
}
pData->map = (char *)data + length;

View File

@ -29,7 +29,7 @@
#include "unicode/udata.h"
#include "putilimp.h"
U_CFUNC UBool uprv_mapFile(UDataMemory *pdm, const char *path);
U_CFUNC UBool uprv_mapFile(UDataMemory *pdm, const char *path, UErrorCode *status);
U_CFUNC void uprv_unmapFile(UDataMemory *pData);
/* MAP_NONE: no memory mapping, no file access at all */

View File

@ -367,7 +367,12 @@ static UResourceDataEntry *init_entry(const char *localeID, const char *path, UE
/* this is the actual loading */
res_load(&(r->fData), r->fPath, r->fName, status);
if (U_FAILURE(*status)) {
if (U_FAILURE(*status)) {
/* if we failed to load due to an out-of-memory error, exit early. */
if (*status == U_MEMORY_ALLOCATION_ERROR) {
uprv_free(r);
return NULL;
}
/* we have no such entry in dll, so it will always use fallback */
*status = U_USING_FALLBACK_WARNING;
r->fBogus = U_USING_FALLBACK_WARNING;
@ -537,6 +542,11 @@ loadParentsExceptRoot(UResourceDataEntry *&t1,
UErrorCode usrStatus = U_ZERO_ERROR;
if (usingUSRData) { // This code inserts user override data into the inheritance chain.
u2 = init_entry(name, usrDataPath, &usrStatus);
// If we failed due to out-of-memory, report that to the caller and exit early.
if (usrStatus == U_MEMORY_ALLOCATION_ERROR) {
*status = usrStatus;
return FALSE;
}
}
if (usingUSRData && U_SUCCESS(usrStatus) && u2->fBogus == U_ZERO_ERROR) {
@ -642,21 +652,32 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID,
/* We're going to skip all the locales that do not have any data */
r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus);
// If we failed due to out-of-memory, report the failure and exit early.
if (intStatus == U_MEMORY_ALLOCATION_ERROR) {
*status = intStatus;
goto finishUnlock;
}
if(r != NULL) { /* if there is one real locale, we can look for parents. */
t1 = r;
hasRealData = TRUE;
if ( usingUSRData ) { /* This code inserts user override data into the inheritance chain */
UErrorCode usrStatus = U_ZERO_ERROR;
UResourceDataEntry *u1 = init_entry(t1->fName, usrDataPath, &usrStatus);
if ( u1 != NULL ) {
if(u1->fBogus == U_ZERO_ERROR) {
u1->fParent = t1;
r = u1;
} else {
/* the USR override data wasn't found, set it to be deleted */
u1->fCountExisting = 0;
}
}
// If we failed due to out-of-memory, report the failure and exit early.
if (intStatus == U_MEMORY_ALLOCATION_ERROR) {
*status = intStatus;
goto finishUnlock;
}
if ( u1 != NULL ) {
if(u1->fBogus == U_ZERO_ERROR) {
u1->fParent = t1;
r = u1;
} else {
/* the USR override data wasn't found, set it to be deleted */
u1->fCountExisting = 0;
}
}
}
if (hasChopped && !isRoot) {
if (!loadParentsExceptRoot(t1, name, UPRV_LENGTHOF(name), usingUSRData, usrDataPath, status)) {
@ -671,6 +692,11 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID,
/* insert default locale */
uprv_strcpy(name, uloc_getDefault());
r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus);
// If we failed due to out-of-memory, report the failure and exit early.
if (intStatus == U_MEMORY_ALLOCATION_ERROR) {
*status = intStatus;
goto finishUnlock;
}
intStatus = U_USING_DEFAULT_WARNING;
if(r != NULL) { /* the default locale exists */
t1 = r;
@ -690,6 +716,11 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID,
if(r == NULL) {
uprv_strcpy(name, kRootLocaleName);
r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus);
// If we failed due to out-of-memory, report the failure and exit early.
if (intStatus == U_MEMORY_ALLOCATION_ERROR) {
*status = intStatus;
goto finishUnlock;
}
if(r != NULL) {
t1 = r;
intStatus = U_USING_DEFAULT_WARNING;