Skip to content

Commit

Permalink
Fix scheduler_id race condition with nif thread specific data
Browse files Browse the repository at this point in the history
  • Loading branch information
ratelle committed Jul 5, 2013
1 parent 9a585d5 commit 9801e3e
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 38 deletions.
2 changes: 1 addition & 1 deletion c_src/geo_normalize.c
Expand Up @@ -69,7 +69,7 @@ geo_normalize(char *in, iconv_t cd)
if (local_cd != NULL)
iconv_close(local_cd);

if (conv == (size_t) -1 && errno == E2BIG) {
if (conv == (size_t) -1) {
// Failed
free(out);
return NULL;
Expand Down
47 changes: 17 additions & 30 deletions c_src/geoip_nif.c
Expand Up @@ -22,30 +22,14 @@ static GeoIP *gip_isp_edition = NULL;
static GeoIP *gip_netspeed_edition = NULL;
static GeoIP *gip_netspeed_edition_rev1 = NULL;

static iconv_t *cds = NULL;
static ErlNifTSDKey iconv_key;

static int
on_load(ErlNifEnv *env, void **priv, ERL_NIF_TERM info)
{
int schedulers;
int i;

if(!enif_get_int(env, info, &schedulers) || schedulers < 1)
if (enif_tsd_key_create("iconv_key", &iconv_key) != 0)
return -1;

cds = (iconv_t*) enif_alloc(schedulers * sizeof(iconv_t));

for (i = 0; i < schedulers; i++) {
cds[i] = iconv_open("ISO-8859-1//IGNORE", "UTF-8");
if (cds[i] == (iconv_t) -1) {
int j;
for(j = 0; j < i; j++)
iconv_close(cds[j]);
enif_free(cds);
return -1;
}
}

if (GeoIP_db_avail(GEOIP_CITY_EDITION_REV1))
gip_city_edition = GeoIP_open_type(GEOIP_CITY_EDITION_REV1,
GEOIP_MODE);
Expand Down Expand Up @@ -77,6 +61,8 @@ on_load(ErlNifEnv *env, void **priv, ERL_NIF_TERM info)
gip_netspeed_edition = GeoIP_open_type(GEOIP_NETSPEED_EDITION,
GEOIP_MODE);

int i;

GeoIP *gis[] = {
gip_country_edition,
gip_region_edition,
Expand Down Expand Up @@ -282,48 +268,49 @@ geo_lookup(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
char *geo_normalize(char *string, iconv_t cd);

static ERL_NIF_TERM
normalize_city_int(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
normalize_city(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{
ErlNifBinary input;
ERL_NIF_TERM retval;
int scheduler_id;
char *in, *out;
iconv_t iconv;

if (argc != 2)
if (argc != 1)
return enif_make_badarg(env);

if (!enif_inspect_iolist_as_binary(env, argv[0], &input))
return enif_make_badarg(env);

if (!enif_get_int(env, argv[1], &scheduler_id) || scheduler_id < 1)
return enif_make_badarg(env);

in = (char *) enif_alloc(input.size+1);
if ((iconv = (iconv_t) enif_tsd_get(iconv_key)) == NULL) {
if ((iconv = iconv_open("ISO-8859-1//IGNORE", "UTF-8")) == (iconv_t) -1)
return make_error(env, "iconv_open_failed");
enif_tsd_set(iconv_key, (void*) iconv);
}

if (in == NULL)
if ((in = enif_alloc(input.size+1)) == NULL)
return make_error(env, "out_of_memory");

memcpy(in, input.data, input.size);
in[input.size] = '\0';

out = geo_normalize(in,cds[scheduler_id-1]);
out = geo_normalize(in,iconv);

if (out == NULL)
retval = make_error(env, "normalization_failed");
else
retval = make_binary_string(env, out);

if (in)
if (in != NULL)
enif_free(in);
if (out)
if (out != NULL)
free(out);

return retval;
}

static ErlNifFunc nif_functions[] = {
{"lookup", 1, geo_lookup},
{"normalize_city_int", 2, normalize_city_int}
{"normalize_city", 1, normalize_city}
};

ERL_NIF_INIT(erlgeoip, nif_functions, &on_load, NULL, NULL, NULL);
2 changes: 1 addition & 1 deletion rebar.config
Expand Up @@ -8,7 +8,7 @@
%% The $DEP variable is created in rebar.config.script
{port_env, [{"GEOIP_LIBS_PATH", "${REBAR_DEPS_DIR}/geoip/libGeoIP/.libs"},
{"GEOIP_LOBJECTS", "${GEOIP_LIBS_PATH}/GeoIP.o ${GEOIP_LIBS_PATH}/GeoIP_depreciated.o ${GEOIP_LIBS_PATH}/GeoIPCity.o ${GEOIP_LIBS_PATH}/regionName.o ${GEOIP_LIBS_PATH}/timeZone.o"},
{"CFLAGS", "$CFLAGS -g -O3 -Wall -fPIC -I${REBAR_DEPS_DIR}/geoip/libGeoIP/"},
{"CFLAGS", "$CFLAGS -g -O2 -Wall -fPIC -I${REBAR_DEPS_DIR}/geoip/libGeoIP/"},
{"DRV_LINK_TEMPLATE",
"$CC $PORT_IN_FILES $GEOIP_LOBJECTS $LDFLAGS $DRV_LDFLAGS -o $PORT_OUT_FILE"}
]}.
Expand Down
8 changes: 2 additions & 6 deletions src/erlgeoip.erl
Expand Up @@ -18,7 +18,7 @@ init() ->
Dir -> Dir
end,
SoName = filename:join(PrivDir, "geoip_nif"),
case catch erlang:load_nif(SoName, erlang:system_info(schedulers)) of
case catch erlang:load_nif(SoName,[]) of
ok -> ok;
LoadError -> error_logger:error_msg("erlgeoip: error loading NIF (~p): ~p",
[SoName, LoadError])
Expand All @@ -27,9 +27,5 @@ init() ->
lookup(_Ip) ->
{error, geoip_nif_not_loaded}.

normalize_city(City) ->
SchedulerId = erlang:system_info(scheduler_id),
normalize_city_int(City,SchedulerId).

normalize_city_int(_City,_SchedulerId) ->
normalize_city(_City) ->
{error, geoip_nif_not_loaded}.

0 comments on commit 9801e3e

Please sign in to comment.