Skip to content

Commit dec0fc0

Browse files
hummeltechhummeltech
authored
Fix macOS mod_tile.c builds (openstreetmap#492)
- Use a local allocator for large data buffers
1 parent f98c835 commit dec0fc0

File tree

2 files changed

+95
-59
lines changed

2 files changed

+95
-59
lines changed

‎.github/workflows/build-and-test.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,12 @@ jobs:
196196
- name: Install dependencies
197197
uses: ./.github/actions/dependencies/install
198198

199-
- name: Set CPATH, ICU_ROOT & LIBRARY_PATH
199+
- name: Set CPATH, ICU_ROOT, LIBRARY_PATH & TEST_PARALLEL_LEVEL
200200
run: |
201201
echo "CPATH=$(brew --prefix)/include" >> ${GITHUB_ENV}
202202
echo "ICU_ROOT=$(brew --prefix icu4c)" >> ${GITHUB_ENV}
203203
echo "LIBRARY_PATH=$(brew --prefix)/lib" >> ${GITHUB_ENV}
204+
echo "TEST_PARALLEL_LEVEL=1" >> ${GITHUB_ENV}
204205
205206
- name: Build `mod_tile`
206207
uses: ./.github/actions/build

‎src/mod_tile.c‎

Lines changed: 93 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,88 @@ static int error_message(request_rec *r, const char *format, ...)
119119
return OK;
120120
}
121121

122+
static apr_pool_t *create_buffer_pool(request_rec *r)
123+
{
124+
apr_allocator_t *allocator;
125+
apr_pool_t *pool;
126+
127+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
128+
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
129+
"Failed to create buffer allocator");
130+
}
131+
132+
apr_allocator_max_free_set(allocator, 1);
133+
134+
if (apr_pool_create_ex(&pool, NULL, NULL, allocator) != APR_SUCCESS) {
135+
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
136+
"Failed to create buffer pool");
137+
apr_allocator_destroy(allocator);
138+
}
139+
140+
apr_allocator_owner_set(allocator, pool);
141+
142+
return pool;
143+
}
144+
145+
static void destroy_buffer_pool(apr_pool_t *pool)
146+
{
147+
apr_pool_destroy(pool);
148+
}
149+
150+
static int get_global_lock(request_rec *r, apr_global_mutex_t *mutex)
151+
{
152+
apr_status_t rs;
153+
int camped;
154+
155+
for (camped = 0; camped < MAXCAMP; camped++) {
156+
rs = apr_global_mutex_trylock(mutex);
157+
158+
if (APR_STATUS_IS_EBUSY(rs)) {
159+
apr_sleep(CAMPOUT);
160+
} else if (rs == APR_SUCCESS) {
161+
return 1;
162+
} else if (APR_STATUS_IS_ENOTIMPL(rs)) {
163+
/* If it's not implemented, just hang in the mutex. */
164+
rs = apr_global_mutex_lock(mutex);
165+
166+
if (rs == APR_SUCCESS) {
167+
return 1;
168+
} else {
169+
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, "Could not get hardlock");
170+
return 0;
171+
}
172+
} else {
173+
ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, "Unknown return status from trylock");
174+
return 0;
175+
}
176+
}
177+
178+
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "Timedout trylock");
179+
return 0;
180+
}
181+
182+
static int get_stats_copy(request_rec *r, tile_server_conf *scfg, stats_data **stats_copy)
183+
{
184+
stats_data *stats;
185+
unsigned long sizeof_config_elements;
186+
187+
if (get_global_lock(r, stats_mutex) != 0) {
188+
// Copy over the global counter variable into
189+
// local variables, that we can immediately
190+
// release the lock again
191+
sizeof_config_elements = sizeof(apr_uint64_t) * scfg->configs->nelts;
192+
stats = (stats_data *)apr_shm_baseaddr_get(stats_shm);
193+
*stats_copy = (stats_data *)apr_pmemdup(r->pool, stats, sizeof(stats_data));
194+
(*stats_copy)->noResp200Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp200Layer, sizeof_config_elements);
195+
(*stats_copy)->noResp404Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp404Layer, sizeof_config_elements);
196+
apr_global_mutex_unlock(stats_mutex);
197+
} else {
198+
return error_message(r, "Failed to acquire lock, can't copy stats");
199+
}
200+
201+
return OK;
202+
}
203+
122204
static int socket_init(request_rec *r)
123205
{
124206
struct addrinfo hints;
@@ -556,38 +638,6 @@ static void add_expiry(request_rec *r, struct protocol *cmd)
556638
apr_table_setn(t, "Expires", timestr);
557639
}
558640

559-
static int get_global_lock(request_rec *r, apr_global_mutex_t *mutex)
560-
{
561-
apr_status_t rs;
562-
int camped;
563-
564-
for (camped = 0; camped < MAXCAMP; camped++) {
565-
rs = apr_global_mutex_trylock(mutex);
566-
567-
if (APR_STATUS_IS_EBUSY(rs)) {
568-
apr_sleep(CAMPOUT);
569-
} else if (rs == APR_SUCCESS) {
570-
return 1;
571-
} else if (APR_STATUS_IS_ENOTIMPL(rs)) {
572-
/* If it's not implemented, just hang in the mutex. */
573-
rs = apr_global_mutex_lock(mutex);
574-
575-
if (rs == APR_SUCCESS) {
576-
return 1;
577-
} else {
578-
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, "Could not get hardlock");
579-
return 0;
580-
}
581-
} else {
582-
ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, "Unknown return status from trylock");
583-
return 0;
584-
}
585-
}
586-
587-
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "Timedout trylock");
588-
return 0;
589-
}
590-
591641
static int incRespCounter(int resp, request_rec *r, struct protocol *cmd, int layerNumber)
592642
{
593643
stats_data *stats;
@@ -1301,7 +1351,8 @@ static int tile_handler_json(request_rec *r)
13011351
}
13021352
}
13031353

1304-
buf = (char *)apr_pcalloc(r->pool, 8 * 1024);
1354+
apr_pool_t *pool = create_buffer_pool(r);
1355+
buf = (char *)apr_pcalloc(pool, 8 * 1024);
13051356

13061357
snprintf(buf, 8 * 1024,
13071358
"{\n"
@@ -1347,28 +1398,7 @@ static int tile_handler_json(request_rec *r)
13471398
apr_rfc822_date(timestr, (apr_time_from_sec(maxAge) + r->request_time));
13481399
apr_table_setn(t, "Expires", timestr);
13491400
ap_rwrite(buf, len, r);
1350-
1351-
return OK;
1352-
}
1353-
1354-
static int _get_stats_copy(request_rec *r, tile_server_conf *scfg, stats_data **stats_copy)
1355-
{
1356-
stats_data *stats;
1357-
unsigned long sizeof_config_elements;
1358-
1359-
if (get_global_lock(r, stats_mutex) != 0) {
1360-
// Copy over the global counter variable into
1361-
// local variables, that we can immediately
1362-
// release the lock again
1363-
sizeof_config_elements = sizeof(apr_uint64_t) * scfg->configs->nelts;
1364-
stats = (stats_data *)apr_shm_baseaddr_get(stats_shm);
1365-
*stats_copy = (stats_data *)apr_pmemdup(r->pool, stats, sizeof(stats_data));
1366-
(*stats_copy)->noResp200Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp200Layer, sizeof_config_elements);
1367-
(*stats_copy)->noResp404Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp404Layer, sizeof_config_elements);
1368-
apr_global_mutex_unlock(stats_mutex);
1369-
} else {
1370-
return error_message(r, "Failed to acquire lock, can't copy stats");
1371-
}
1401+
destroy_buffer_pool(pool);
13721402

13731403
return OK;
13741404
}
@@ -1391,7 +1421,7 @@ static int tile_handler_mod_stats(request_rec *r)
13911421
return error_message(r, "Stats are not enabled for this server");
13921422
}
13931423

1394-
_get_stats_copy(r, scfg, &local_stats);
1424+
get_stats_copy(r, scfg, &local_stats);
13951425

13961426
if (!local_stats) {
13971427
return OK;
@@ -1449,7 +1479,7 @@ static int tile_handler_metrics(request_rec *r)
14491479
return error_message(r, "Stats are not enabled for this server");
14501480
}
14511481

1452-
_get_stats_copy(r, scfg, &local_stats);
1482+
get_stats_copy(r, scfg, &local_stats);
14531483

14541484
if (!local_stats) {
14551485
return OK;
@@ -1556,7 +1586,8 @@ static int tile_handler_serve(request_rec *r)
15561586
gettimeofday(&start, NULL);
15571587

15581588
// FIXME: It is a waste to do the malloc + read if we are fulfilling a HEAD or returning a 304.
1559-
buf = (char *)apr_pcalloc(r->pool, tile_max);
1589+
apr_pool_t *pool = create_buffer_pool(r);
1590+
buf = (char *)apr_pcalloc(pool, tile_max);
15601591

15611592
if (!buf) {
15621593
if (!incRespCounter(HTTP_INTERNAL_SERVER_ERROR, r, cmd, rdata->layerNumber)) {
@@ -1607,6 +1638,8 @@ static int tile_handler_serve(request_rec *r)
16071638
incTimingCounter((end.tv_sec * 1000000 + end.tv_usec) - (start.tv_sec * 1000000 + start.tv_usec), cmd->z, r);
16081639

16091640
if ((errstatus = ap_meets_conditions(r)) != OK) {
1641+
destroy_buffer_pool(pool);
1642+
16101643
if (!incRespCounter(errstatus, r, cmd, rdata->layerNumber)) {
16111644
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
16121645
"Failed to increase response stats counter");
@@ -1615,6 +1648,7 @@ static int tile_handler_serve(request_rec *r)
16151648
return errstatus;
16161649
} else {
16171650
ap_rwrite(buf, len, r);
1651+
destroy_buffer_pool(pool);
16181652

16191653
if (!incRespCounter(errstatus, r, cmd, rdata->layerNumber)) {
16201654
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
@@ -1625,6 +1659,7 @@ static int tile_handler_serve(request_rec *r)
16251659
}
16261660
}
16271661

1662+
destroy_buffer_pool(pool);
16281663
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, "Failed to read tile from disk: %s", err_msg);
16291664

16301665
if (!incRespCounter(HTTP_NOT_FOUND, r, cmd, rdata->layerNumber)) {

0 commit comments

Comments
 (0)