Skip to content

Commit a73f9cc

Browse files
committed
Prevent memory leaks from rb_raise
rb_raise calls longjmp, which bypasses C++ destructors, and also keeps the error for catch blocks from being unallocated if passed by reference, which we do for exceptions. Some of the calls I left can still jump out of try blocks, which you're not supposed to do, but there shouldn't be any objects with destructors initialized at those points so it's probably fine.
1 parent 1462dc9 commit a73f9cc

18 files changed

+496
-357
lines changed

binding/audio-binding.cpp

+16-11
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,18 @@
2525
#include "exception.h"
2626

2727
#define DEF_PLAY_STOP_POS(entity) \
28-
RB_METHOD(audio_##entity##Play) \
28+
RB_METHOD_GUARD(audio_##entity##Play) \
2929
{ \
3030
RB_UNUSED_PARAM; \
3131
const char *filename; \
3232
int volume = 100; \
3333
int pitch = 100; \
3434
double pos = 0.0; \
35-
rb_get_args(argc, argv, "z|iif", &filename, &volume, &pitch, &pos RB_ARG_END); \
36-
GUARD_EXC( shState->audio().entity##Play(filename, volume, pitch, pos); ) \
35+
rb_get_args(argc, argv, "z|iif", &filename, &volume, &pitch, &pos RB_ARG_END); \
36+
shState->audio().entity##Play(filename, volume, pitch, pos); \
3737
return Qnil; \
3838
} \
39+
RB_METHOD_GUARD_END \
3940
RB_METHOD(audio_##entity##Stop) \
4041
{ \
4142
RB_UNUSED_PARAM; \
@@ -49,16 +50,17 @@
4950
}
5051

5152
#define DEF_PLAY_STOP(entity) \
52-
RB_METHOD(audio_##entity##Play) \
53+
RB_METHOD_GUARD(audio_##entity##Play) \
5354
{ \
5455
RB_UNUSED_PARAM; \
5556
const char *filename; \
5657
int volume = 100; \
5758
int pitch = 100; \
5859
rb_get_args(argc, argv, "z|ii", &filename, &volume, &pitch RB_ARG_END); \
59-
GUARD_EXC( shState->audio().entity##Play(filename, volume, pitch); ) \
60+
shState->audio().entity##Play(filename, volume, pitch); \
6061
return Qnil; \
6162
} \
63+
RB_METHOD_GUARD_END \
6264
RB_METHOD(audio_##entity##Stop) \
6365
{ \
6466
RB_UNUSED_PARAM; \
@@ -87,7 +89,7 @@ RB_METHOD(audio_##entity##Fade) \
8789

8890
#define MAYBE_NIL_TRACK(t) t == Qnil ? -127 : NUM2INT(t)
8991

90-
RB_METHOD(audio_bgmPlay)
92+
RB_METHOD_GUARD(audio_bgmPlay)
9193
{
9294
RB_UNUSED_PARAM;
9395
const char *filename;
@@ -96,9 +98,10 @@ RB_METHOD(audio_bgmPlay)
9698
double pos = 0.0;
9799
VALUE track = Qnil;
98100
rb_get_args(argc, argv, "z|iifo", &filename, &volume, &pitch, &pos, &track RB_ARG_END);
99-
GUARD_EXC( shState->audio().bgmPlay(filename, volume, pitch, pos, MAYBE_NIL_TRACK(track)); )
101+
shState->audio().bgmPlay(filename, volume, pitch, pos, MAYBE_NIL_TRACK(track));
100102
return Qnil;
101103
}
104+
RB_METHOD_GUARD_END
102105

103106
RB_METHOD(audio_bgmStop)
104107
{
@@ -117,25 +120,27 @@ RB_METHOD(audio_bgmPos)
117120
return rb_float_new(shState->audio().bgmPos(MAYBE_NIL_TRACK(track)));
118121
}
119122

120-
RB_METHOD(audio_bgmGetVolume)
123+
RB_METHOD_GUARD(audio_bgmGetVolume)
121124
{
122125
RB_UNUSED_PARAM;
123126
VALUE track = Qnil;
124127
rb_get_args(argc, argv, "|o", &track RB_ARG_END);
125128
int ret = 0;
126-
GUARD_EXC( ret = shState->audio().bgmGetVolume(MAYBE_NIL_TRACK(track)); )
129+
ret = shState->audio().bgmGetVolume(MAYBE_NIL_TRACK(track));
127130
return rb_fix_new(ret);
128131
}
132+
RB_METHOD_GUARD_END
129133

130-
RB_METHOD(audio_bgmSetVolume)
134+
RB_METHOD_GUARD(audio_bgmSetVolume)
131135
{
132136
RB_UNUSED_PARAM;
133137
int volume;
134138
VALUE track = Qnil;
135139
rb_get_args(argc, argv, "i|o", &volume, &track RB_ARG_END);
136-
GUARD_EXC( shState->audio().bgmSetVolume(volume, MAYBE_NIL_TRACK(track)); )
140+
shState->audio().bgmSetVolume(volume, MAYBE_NIL_TRACK(track));
137141
return Qnil;
138142
}
143+
RB_METHOD_GUARD_END
139144

140145
DEF_PLAY_STOP_POS( bgs )
141146

binding/binding-mri.cpp

+30-29
Original file line numberDiff line numberDiff line change
@@ -528,14 +528,15 @@ RB_METHOD(mkxpSystemMemory) {
528528
return INT2NUM(SDL_GetSystemRAM());
529529
}
530530

531-
RB_METHOD(mkxpReloadPathCache) {
531+
RB_METHOD_GUARD(mkxpReloadPathCache) {
532532
RB_UNUSED_PARAM;
533533

534-
GUARD_EXC(shState->fileSystem().reloadPathCache(););
534+
shState->fileSystem().reloadPathCache();
535535
return Qnil;
536536
}
537+
RB_METHOD_GUARD_END
537538

538-
RB_METHOD(mkxpAddPath) {
539+
RB_METHOD_GUARD(mkxpAddPath) {
539540
RB_UNUSED_PARAM;
540541

541542
VALUE path, mountpoint, reload;
@@ -545,36 +546,32 @@ RB_METHOD(mkxpAddPath) {
545546

546547
const char *mp = (mountpoint == Qnil) ? 0 : RSTRING_PTR(mountpoint);
547548

548-
try {
549-
bool rl = true;
550-
if (reload != Qnil)
551-
rb_bool_arg(reload, &rl);
552-
553-
shState->fileSystem().addPath(RSTRING_PTR(path), mp, rl);
554-
} catch (Exception &e) {
555-
raiseRbExc(e);
556-
}
549+
bool rl = true;
550+
if (reload != Qnil)
551+
rb_bool_arg(reload, &rl);
552+
553+
shState->fileSystem().addPath(RSTRING_PTR(path), mp, rl);
554+
557555
return path;
558556
}
557+
RB_METHOD_GUARD_END
559558

560-
RB_METHOD(mkxpRemovePath) {
559+
RB_METHOD_GUARD(mkxpRemovePath) {
561560
RB_UNUSED_PARAM;
562561

563562
VALUE path, reload;
564563
rb_scan_args(argc, argv, "11", &path, &reload);
565564
SafeStringValue(path);
566565

567-
try {
568-
bool rl = true;
569-
if (reload != Qnil)
570-
rb_bool_arg(reload, &rl);
571-
572-
shState->fileSystem().removePath(RSTRING_PTR(path), rl);
573-
} catch (Exception &e) {
574-
raiseRbExc(e);
575-
}
566+
bool rl = true;
567+
if (reload != Qnil)
568+
rb_bool_arg(reload, &rl);
569+
570+
shState->fileSystem().removePath(RSTRING_PTR(path), rl);
571+
576572
return path;
577573
}
574+
RB_METHOD_GUARD_END
578575

579576
RB_METHOD(mkxpFileExists) {
580577
RB_UNUSED_PARAM;
@@ -601,24 +598,25 @@ RB_METHOD(mkxpSetDefaultFontFamily) {
601598
return Qnil;
602599
}
603600

604-
RB_METHOD(mkxpStringToUTF8) {
601+
RB_METHOD_GUARD(mkxpStringToUTF8) {
605602
RB_UNUSED_PARAM;
606603

607604
rb_check_argc(argc, 0);
608605

609606
std::string ret(RSTRING_PTR(self), RSTRING_LEN(self));
610-
GUARD_EXC(ret = Encoding::convertString(ret); );
607+
ret = Encoding::convertString(ret);
611608

612609
return rb_utf8_str_new(ret.c_str(), ret.length());
613610
}
611+
RB_METHOD_GUARD_END
614612

615-
RB_METHOD(mkxpStringToUTF8Bang) {
613+
RB_METHOD_GUARD(mkxpStringToUTF8Bang) {
616614
RB_UNUSED_PARAM;
617615

618616
rb_check_argc(argc, 0);
619617

620618
std::string ret(RSTRING_PTR(self), RSTRING_LEN(self));
621-
GUARD_EXC(ret = Encoding::convertString(ret); );
619+
ret = Encoding::convertString(ret);
622620

623621
rb_str_resize(self, ret.length());
624622
memcpy(RSTRING_PTR(self), ret.c_str(), RSTRING_LEN(self));
@@ -629,6 +627,7 @@ RB_METHOD(mkxpStringToUTF8Bang) {
629627

630628
return self;
631629
}
630+
RB_METHOD_GUARD_END
632631

633632
#ifdef __APPLE__
634633
#define OPENCMD "open "
@@ -641,7 +640,7 @@ RB_METHOD(mkxpStringToUTF8Bang) {
641640
#define OPENARGS ""
642641
#endif
643642

644-
RB_METHOD(mkxpLaunch) {
643+
RB_METHOD_GUARD(mkxpLaunch) {
645644
RB_UNUSED_PARAM;
646645

647646
VALUE cmdname, args;
@@ -674,11 +673,12 @@ RB_METHOD(mkxpLaunch) {
674673
}
675674

676675
if (std::system(command.c_str()) != 0) {
677-
raiseRbExc(Exception(Exception::MKXPError, "Failed to launch \"%s\"", RSTRING_PTR(cmdname)));
676+
throw Exception(Exception::MKXPError, "Failed to launch \"%s\"", RSTRING_PTR(cmdname));
678677
}
679678

680679
return RUBY_Qnil;
681680
}
681+
RB_METHOD_GUARD_END
682682

683683
json5pp::value loadUserSettings() {
684684
json5pp::value ret;
@@ -722,7 +722,7 @@ RB_METHOD(mkxpGetJSONSetting) {
722722

723723
}
724724

725-
RB_METHOD(mkxpSetJSONSetting) {
725+
RB_METHOD_GUARD(mkxpSetJSONSetting) {
726726
RB_UNUSED_PARAM;
727727

728728
VALUE sname, svalue;
@@ -736,6 +736,7 @@ RB_METHOD(mkxpSetJSONSetting) {
736736

737737
return Qnil;
738738
}
739+
RB_METHOD_GUARD_END
739740

740741
RB_METHOD(mkxpGetAllJSONSettings) {
741742
RB_UNUSED_PARAM;

0 commit comments

Comments
 (0)