Skip to content

Commit ba6da1a

Browse files
committed
Fix memory leaks when returning Neo4j strings
When creating new Perl values through API functions like newSVpv() or hv_store(), const char* values are internally copied into memory that is managed by Perl itself. Manually allocating a buffer isn't necessary. Our neo4j_string_to_alloc_str() function does allocate a buffer and never frees it, causing a leak. neo4j_ustring_value() should be used instead. However, neo4j_ustring_value() currently doesn't return a valid pointer for empty strings, so we need to special-case that. (majensen/libneo4j-omni#10) When returning SVs from an XSUB, assigning them to a Perl variable will increment their ref count, which causes a leak unless the SV on the stack is mortal. If the SV* typemap is used, they become mortal automatically when they are returned. However, when instead pushing SVs onto the value stack manually, the SVs need to be mortalised manually as well.
1 parent 4e5e932 commit ba6da1a

File tree

2 files changed

+5
-15
lines changed

2 files changed

+5
-15
lines changed

lib/Neo4j/Bolt/CTypeHandlers.xs

+4-14
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,6 @@ HV* neo4j_map_to_HV( neo4j_value_t value );
4646
SV* neo4j_value_to_SV( neo4j_value_t value );
4747

4848
long long neo4j_identity_value(neo4j_value_t value);
49-
char *neo4j_string_to_alloc_str(neo4j_value_t value);
50-
51-
char *neo4j_string_to_alloc_str(neo4j_value_t value) {
52-
assert(neo4j_type(value) == NEO4J_STRING);
53-
char *s;
54-
int nlength;
55-
nlength = (int) neo4j_string_length(value);
56-
Newx(s,nlength+1,char);
57-
return neo4j_string_value(value,s,(size_t) nlength+1);
58-
}
5949

6050
neo4j_value_t SViv_to_neo4j_bool (SV *sv) {
6151
return neo4j_bool( (bool) SvIV(sv) );
@@ -309,7 +299,7 @@ SV* neo4j_string_to_SVpv( neo4j_value_t value ) {
309299
STRLEN len;
310300
SV* pv;
311301
len = neo4j_string_length(value);
312-
pv = newSVpvn(neo4j_string_to_alloc_str(value), len);
302+
pv = len ? newSVpvn(neo4j_ustring_value(value), len) : newSVpvs("");
313303
sv_utf8_decode(pv);
314304
return pv;
315305
}
@@ -365,18 +355,18 @@ AV* neo4j_list_to_AV( neo4j_value_t value ) {
365355
HV* neo4j_map_to_HV( neo4j_value_t value ) {
366356
int i,n;
367357
I32 klen;
368-
char *ks;
358+
const char *ks;
369359
const neo4j_map_entry_t *entry;
370360
HV *hv;
371361
SV *sv;
372362
hv = newHV();
373363
n = (int) neo4j_map_size(value);
374364
for (i=0;i<n;i++) {
375365
entry = neo4j_map_getentry(value,i);
376-
ks = neo4j_string_to_alloc_str(entry->key);
366+
klen = neo4j_string_length(entry->key);
367+
ks = klen ? neo4j_ustring_value(entry->key) : "";
377368
sv = neo4j_value_to_SV(entry->value);
378369
SvREFCNT_inc(sv);
379-
klen = neo4j_string_length(entry->key);
380370
if (! is_ascii_string((U8 *)ks, (STRLEN)klen)) {
381371
// treat key as utf8 (as opposed to single-byte)
382372
klen = -klen;

lib/Neo4j/Bolt/ResultStream.xs

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ void fetch_next_ (SV *rs_ref) {
4747
for (i=0; i<n; i++) {
4848
value = neo4j_result_field(result, i);
4949
perl_value = neo4j_value_to_SV(value);
50-
Inline_Stack_Push( perl_value );
50+
Inline_Stack_Push( sv_2mortal(perl_value) );
5151
}
5252
Inline_Stack_Done;
5353
return;

0 commit comments

Comments
 (0)