Skip to content

Commit e17e65d

Browse files
yatsukhnenkoyatsukhnenko
committed
Use zend_string for all lock_status members
1 parent 07c16e5 commit e17e65d

File tree

2 files changed

+42
-45
lines changed

2 files changed

+42
-45
lines changed

‎common.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ zend_string_init(const char *str, size_t len, int persistent)
4646
return zstr;
4747
}
4848

49+
#define zend_string_equal_val(s1, s2) !memcmp(ZSTR_VAL(s1), ZSTR_VAL(s2), ZSTR_LEN(s1))
50+
#define zend_string_equal_content(s1, s2) (ZSTR_LEN(s1) == ZSTR_LEN(s2) && zend_string_equal_val(s1, s2))
51+
#define zend_string_equals(s1, s2) (s1 == s2 || zend_string_equal_content(s1, s2))
52+
4953
#define zend_string_release(s) do { \
5054
if ((s) && (s)->gc) { \
5155
if ((s)->gc & 0x10 && ZSTR_VAL(s)) efree(ZSTR_VAL(s)); \

‎redis_session.c

Lines changed: 38 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ ps_module ps_mod_redis_cluster = {
7070

7171
typedef struct {
7272
zend_bool is_locked;
73-
char *session_key;
74-
char *lock_key;
75-
char *lock_secret;
73+
zend_string *session_key;
74+
zend_string *lock_key;
75+
zend_string *lock_secret;
7676
} redis_session_lock_status;
7777

7878
typedef struct redis_pool_member_ {
@@ -130,12 +130,9 @@ redis_pool_free(redis_pool *pool TSRMLS_DC) {
130130
}
131131

132132
/* Cleanup after our lock */
133-
if (pool->lock_status.session_key)
134-
efree(pool->lock_status.session_key);
135-
if (pool->lock_status.lock_secret)
136-
efree(pool->lock_status.lock_secret);
137-
if (pool->lock_status.lock_key)
138-
efree(pool->lock_status.lock_key);
133+
if (pool->lock_status.session_key) zend_string_release(pool->lock_status.session_key);
134+
if (pool->lock_status.lock_secret) zend_string_release(pool->lock_status.lock_secret);
135+
if (pool->lock_status.lock_key) zend_string_release(pool->lock_status.lock_key);
139136

140137
/* Cleanup pool itself */
141138
efree(pool);
@@ -247,7 +244,7 @@ static int set_session_lock_key(RedisSock *redis_sock, char *cmd, int cmd_len
247244
static int lock_acquire(RedisSock *redis_sock, redis_session_lock_status *lock_status
248245
TSRMLS_DC)
249246
{
250-
char *cmd, hostname[HOST_NAME_MAX] = {0};
247+
char *cmd, hostname[HOST_NAME_MAX] = {0}, suffix[] = "_LOCK", pid[32];
251248
int cmd_len, lock_wait_time, retries, i, expiry;
252249

253250
/* Short circuit if we are already locked or not using session locks */
@@ -273,21 +270,25 @@ static int lock_acquire(RedisSock *redis_sock, redis_session_lock_status *lock_s
273270
}
274271

275272
/* Generate our qualified lock key */
276-
spprintf(&lock_status->lock_key, 0, "%s%s", lock_status->session_key, "_LOCK");
273+
lock_status->lock_key = zend_string_alloc(ZSTR_LEN(lock_status->session_key) + sizeof(suffix) - 1, 0);
274+
memcpy(ZSTR_VAL(lock_status->lock_key), ZSTR_VAL(lock_status->session_key), ZSTR_LEN(lock_status->session_key));
275+
memcpy(ZSTR_VAL(lock_status->lock_key) + ZSTR_LEN(lock_status->session_key), suffix, sizeof(suffix) - 1);
277276

278277
/* Calculate lock secret */
279278
gethostname(hostname, HOST_NAME_MAX);
280-
spprintf(&lock_status->lock_secret, 0, "%s|%ld", hostname, (long)getpid());
279+
size_t hostname_len = strlen(hostname);
280+
size_t pid_len = snprintf(pid, sizeof(pid), "|%ld", (long)getpid());
281+
lock_status->lock_secret = zend_string_alloc(hostname_len + pid_len, 0);
282+
memcpy(ZSTR_VAL(lock_status->lock_secret), hostname, hostname_len);
283+
memcpy(ZSTR_VAL(lock_status->lock_secret) + hostname_len, pid, pid_len);
281284

282285
if (expiry > 0) {
283-
cmd_len = REDIS_SPPRINTF(&cmd, "SET", "ssssd", lock_status->lock_key,
284-
strlen(lock_status->lock_key), lock_status->lock_secret,
285-
strlen(lock_status->lock_secret), "NX", 2,
286-
"PX", 2, expiry * 1000);
286+
cmd_len = REDIS_SPPRINTF(&cmd, "SET", "SSssd", lock_status->lock_key,
287+
lock_status->lock_secret, "NX", 2, "PX", 2,
288+
expiry * 1000);
287289
} else {
288-
cmd_len = REDIS_SPPRINTF(&cmd, "SET", "sss", lock_status->lock_key,
289-
strlen(lock_status->lock_key), lock_status->lock_secret,
290-
strlen(lock_status->lock_secret), "NX", 2);
290+
cmd_len = REDIS_SPPRINTF(&cmd, "SET", "SSs", lock_status->lock_key,
291+
lock_status->lock_secret, "NX", 2);
291292
}
292293

293294
/* Attempt to get our lock */
@@ -310,7 +311,7 @@ static int lock_acquire(RedisSock *redis_sock, redis_session_lock_status *lock_s
310311
return lock_status->is_locked ? SUCCESS : FAILURE;
311312
}
312313

313-
#define IS_LOCK_SECRET(reply, len, secret) (len == strlen(secret) && !strncmp(reply, secret, len))
314+
#define IS_LOCK_SECRET(reply, len, secret) (len == ZSTR_LEN(secret) && !strncmp(reply, ZSTR_VAL(secret), len))
314315
static void refresh_lock_status(RedisSock *redis_sock, redis_session_lock_status *lock_status TSRMLS_DC)
315316
{
316317
char *cmd, *reply = NULL;
@@ -327,8 +328,7 @@ static void refresh_lock_status(RedisSock *redis_sock, redis_session_lock_status
327328
return;
328329

329330
/* Command to get our lock key value and compare secrets */
330-
cmdlen = REDIS_SPPRINTF(&cmd, "GET", "s", lock_status->lock_key,
331-
strlen(lock_status->lock_key));
331+
cmdlen = REDIS_SPPRINTF(&cmd, "GET", "S", lock_status->lock_key);
332332

333333
/* Attempt to refresh the lock */
334334
reply = redis_simple_cmd(redis_sock, cmd, cmdlen, &replylen TSRMLS_CC);
@@ -377,9 +377,8 @@ static void lock_release(RedisSock *redis_sock, redis_session_lock_status *lock_
377377
/* We first want to try EVALSHA and then fall back to EVAL */
378378
for (i = 0; lock_status->is_locked && i < sizeof(kwd)/sizeof(*kwd); i++) {
379379
/* Construct our command */
380-
cmdlen = REDIS_SPPRINTF(&cmd, (char*)kwd[i], "sdss", lua[i], len[i], 1,
381-
lock_status->lock_key, strlen(lock_status->lock_key),
382-
lock_status->lock_secret, strlen(lock_status->lock_secret));
380+
cmdlen = REDIS_SPPRINTF(&cmd, (char*)kwd[i], "sdSS", lua[i], len[i], 1,
381+
lock_status->lock_key, lock_status->lock_secret);
383382

384383
/* Send it off */
385384
reply = redis_simple_cmd(redis_sock, cmd, cmdlen, &replylen TSRMLS_CC);
@@ -538,7 +537,7 @@ PS_CLOSE_FUNC(redis)
538537
redis_pool *pool = PS_GET_MOD_DATA();
539538

540539
if (pool) {
541-
redis_pool_member *rpm = redis_pool_get_sock(pool, pool->lock_status.session_key TSRMLS_CC);
540+
redis_pool_member *rpm = redis_pool_get_sock(pool, ZSTR_VAL(pool->lock_status.session_key) TSRMLS_CC);
542541

543542
RedisSock *redis_sock = rpm ? rpm->redis_sock : NULL;
544543
if (redis_sock) {
@@ -613,18 +612,15 @@ PS_CREATE_SID_FUNC(redis)
613612
}
614613

615614
#if (PHP_MAJOR_VERSION < 7)
616-
zend_string *session = redis_session_key(rpm, sid, strlen(sid));
615+
pool->lock_status.session_key = redis_session_key(rpm, sid, strlen(sid));
617616
#else
618-
zend_string *session = redis_session_key(rpm, ZSTR_VAL(sid), ZSTR_LEN(sid));
617+
pool->lock_status.session_key = redis_session_key(rpm, ZSTR_VAL(sid), ZSTR_LEN(sid));
619618
#endif
620-
pool->lock_status.session_key = estrndup(ZSTR_VAL(session), ZSTR_LEN(session));
621-
zend_string_release(session);
622-
623619
if (lock_acquire(redis_sock, &pool->lock_status TSRMLS_CC) == SUCCESS) {
624620
return sid;
625621
}
626622

627-
efree(pool->lock_status.session_key);
623+
zend_string_release(pool->lock_status.session_key);
628624
#if (PHP_MAJOR_VERSION < 7)
629625
efree(sid);
630626
#else
@@ -664,10 +660,8 @@ PS_READ_FUNC(redis)
664660
}
665661

666662
/* send GET command */
667-
zend_string *session = redis_session_key(rpm, skey, skeylen);
668-
pool->lock_status.session_key = estrndup(ZSTR_VAL(session), ZSTR_LEN(session));
669-
cmd_len = REDIS_SPPRINTF(&cmd, "GET", "S", session);
670-
zend_string_release(session);
663+
pool->lock_status.session_key = redis_session_key(rpm, skey, skeylen);
664+
cmd_len = REDIS_SPPRINTF(&cmd, "GET", "S", pool->lock_status.session_key);
671665

672666
if (lock_acquire(redis_sock, &pool->lock_status TSRMLS_CC) != SUCCESS) {
673667
php_error_docref(NULL TSRMLS_CC, E_NOTICE,
@@ -734,15 +728,14 @@ PS_WRITE_FUNC(redis)
734728
zend_string *session = redis_session_key(rpm, skey, skeylen);
735729
#if (PHP_MAJOR_VERSION < 7)
736730
/* We need to check for PHP5 if the session key changes (a bug with session_regenerate_id() is causing a missing PS_CREATE_SID call)*/
737-
int session_key_changed = strlen(pool->lock_status.session_key) != ZSTR_LEN(session) || strncmp(pool->lock_status.session_key, ZSTR_VAL(session), ZSTR_LEN(session)) != 0;
738-
if (session_key_changed) {
739-
efree(pool->lock_status.session_key);
740-
pool->lock_status.session_key = estrndup(ZSTR_VAL(session), ZSTR_LEN(session));
741-
}
742-
743-
if (session_key_changed && lock_acquire(redis_sock, &pool->lock_status TSRMLS_CC) != SUCCESS) {
744-
zend_string_release(session);
745-
return FAILURE;
731+
if (!zend_string_equals(pool->lock_status.session_key, session)) {
732+
zend_string_release(pool->lock_status.session_key);
733+
pool->lock_status.session_key = zend_string_init(ZSTR_VAL(session), ZSTR_LEN(session), 0);
734+
if (lock_acquire(redis_sock, &pool->lock_status TSRMLS_CC) != SUCCESS) {
735+
zend_string_release(pool->lock_status.session_key);
736+
zend_string_release(session);
737+
return FAILURE;
738+
}
746739
}
747740
#endif
748741
cmd_len = REDIS_SPPRINTF(&cmd, "SETEX", "Sds", session, INI_INT("session.gc_maxlifetime"), sval, svallen);

0 commit comments

Comments
 (0)