[Scummvm-git-logs] scummvm master -> a6659ba9d577998139cbb9a61532f823a24bc3b9

csnover csnover at users.noreply.github.com
Mon Nov 13 06:15:21 CET 2017


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
0eda63bed1 SCUMM: Fix race condition in MOD player
a6659ba9d5 DREAMWEB: Use accurate memory reclamation for Ex transfers


Commit: 0eda63bed1b2d7fe0dba1360cad880b826248eaa
    https://github.com/scummvm/scummvm/commit/0eda63bed1b2d7fe0dba1360cad880b826248eaa
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-11-12T23:15:05-06:00

Commit Message:
SCUMM: Fix race condition in MOD player

Fixes Trac#6272.

Changed paths:
    engines/scumm/players/player_mod.cpp
    engines/scumm/players/player_mod.h


diff --git a/engines/scumm/players/player_mod.cpp b/engines/scumm/players/player_mod.cpp
index ced30ff..d780df9 100644
--- a/engines/scumm/players/player_mod.cpp
+++ b/engines/scumm/players/player_mod.cpp
@@ -63,11 +63,13 @@ void Player_MOD::setMusicVolume(int vol) {
 }
 
 void Player_MOD::setUpdateProc(ModUpdateProc *proc, void *param, int freq) {
+	Common::StackLock lock(_mutex);
 	_playproc = proc;
 	_playparam = param;
 	_mixamt = _sampleRate / freq;
 }
 void Player_MOD::clearUpdateProc() {
+	Common::StackLock lock(_mutex);
 	_playproc = NULL;
 	_playparam = NULL;
 	_mixamt = 0;
@@ -78,6 +80,7 @@ void Player_MOD::startChannel(int id, void *data, int size, int rate, uint8 vol,
 	if (id == 0)
 		error("player_mod - attempted to start channel id 0");
 
+	Common::StackLock lock(_mutex);
 	for (i = 0; i < MOD_MAXCHANS; i++) {
 		if (!_channels[i].id)
 			break;
@@ -106,6 +109,8 @@ void Player_MOD::startChannel(int id, void *data, int size, int rate, uint8 vol,
 void Player_MOD::stopChannel(int id) {
 	if (id == 0)
 		error("player_mod - attempted to stop channel id 0");
+
+	Common::StackLock lock(_mutex);
 	for (int i = 0; i < MOD_MAXCHANS; i++) {
 		if (_channels[i].id == id) {
 			delete _channels[i].input;
@@ -121,6 +126,8 @@ void Player_MOD::stopChannel(int id) {
 void Player_MOD::setChannelVol(int id, uint8 vol) {
 	if (id == 0)
 		error("player_mod - attempted to set volume for channel id 0");
+
+	Common::StackLock lock(_mutex);
 	for (int i = 0; i < MOD_MAXCHANS; i++) {
 		if (_channels[i].id == id) {
 			_channels[i].vol = vol;
@@ -132,6 +139,8 @@ void Player_MOD::setChannelVol(int id, uint8 vol) {
 void Player_MOD::setChannelPan(int id, int8 pan) {
 	if (id == 0)
 		error("player_mod - attempted to set pan for channel id 0");
+
+	Common::StackLock lock(_mutex);
 	for (int i = 0; i < MOD_MAXCHANS; i++) {
 		if (_channels[i].id == id) {
 			_channels[i].pan = pan;
@@ -143,6 +152,8 @@ void Player_MOD::setChannelPan(int id, int8 pan) {
 void Player_MOD::setChannelFreq(int id, int freq) {
 	if (id == 0)
 		error("player_mod - attempted to set frequency for channel id 0");
+
+	Common::StackLock lock(_mutex);
 	for (int i = 0; i < MOD_MAXCHANS; i++) {
 		if (_channels[i].id == id) {
 			if (freq > 31400)	// this is about as high as WinUAE goes
diff --git a/engines/scumm/players/player_mod.h b/engines/scumm/players/player_mod.h
index bb0c422..a157f34 100644
--- a/engines/scumm/players/player_mod.h
+++ b/engines/scumm/players/player_mod.h
@@ -26,6 +26,7 @@
 #include "scumm/scumm.h"
 #include "audio/audiostream.h"
 #include "audio/mixer.h"
+#include "common/mutex.h"
 
 namespace Audio {
 class RateConverter;
@@ -55,6 +56,7 @@ public:
 
 	// AudioStream API
 	int readBuffer(int16 *buffer, const int numSamples) {
+		Common::StackLock lock(_mutex);
 		do_mix(buffer, numSamples / 2);
 		return numSamples;
 	}
@@ -80,6 +82,7 @@ private:
 
 	Audio::Mixer *_mixer;
 	Audio::SoundHandle _soundHandle;
+	Common::Mutex _mutex;
 
 	uint32 _mixamt;
 	uint32 _mixpos;


Commit: a6659ba9d577998139cbb9a61532f823a24bc3b9
    https://github.com/scummvm/scummvm/commit/a6659ba9d577998139cbb9a61532f823a24bc3b9
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-11-12T23:15:05-06:00

Commit Message:
DREAMWEB: Use accurate memory reclamation for Ex transfers

When the Ex memory regions are close to full, it is possible for
the game to fail to purge objects and then crash with an OOM error
even if it isn't actually out of memory. This patch calculates the
amount of free memory truly needed when allocating to Ex memory to
allow exactly the entire frame & text regions to be used, instead
previously where a hard-coded amount of free space to maintain was
used, which guaranteed that the entire memory region could not
actually be used by the game.

This change may be masking some underlying memory leak, or it may
just be that near the end of the game the game naturally comes
close to reaching the maximum memory region size. For the moment,
I am assuming the latter.

This commit also adds some assertion checks to the memory transfer
functions to make sure the regions don't quietly overflow in other
cases, since pickupConts performs transfers in a manner that
doesn't ensure enough free memory exists for them to be successful.

Fixes Trac#6820.

Changed paths:
    engines/dreamweb/dreamweb.h
    engines/dreamweb/object.cpp
    engines/dreamweb/vgagrafx.cpp


diff --git a/engines/dreamweb/dreamweb.h b/engines/dreamweb/dreamweb.h
index 45bacdb..f4a3c00 100644
--- a/engines/dreamweb/dreamweb.h
+++ b/engines/dreamweb/dreamweb.h
@@ -893,7 +893,7 @@ public:
 	void cantDrop();
 	void entryAnims();
 	bool finishedWalking();
-	void emergencyPurge();
+	void emergencyPurge(uint8 from);
 	void purgeAnItem();
 	uint8 nextSymbol(uint8 symbol);
 	void enterSymbol();
diff --git a/engines/dreamweb/object.cpp b/engines/dreamweb/object.cpp
index 181987d..9191702 100644
--- a/engines/dreamweb/object.cpp
+++ b/engines/dreamweb/object.cpp
@@ -253,6 +253,7 @@ void DreamWebEngine::transferText(uint8 from, uint8 to) {
 	char *dst = _exText._text + _vars._exTextPos;
 
 	size_t len = strlen(src);
+	assert(_vars._exTextPos + len + 1 <= kExtextlen);
 	memcpy(dst, src, len + 1);
 	_vars._exTextPos += len + 1;
 }
@@ -1010,7 +1011,7 @@ ObjectRef DreamWebEngine::findOpenPos() {
 }
 
 byte DreamWebEngine::transferToEx(uint8 from) {
-	emergencyPurge();
+	emergencyPurge(from);
 
 	byte pos = getExPos();
 	DynObject *exObject = getExAd(pos);
@@ -1128,11 +1129,18 @@ void DreamWebEngine::incRyanPage() {
 	delPointer();
 }
 
-void DreamWebEngine::emergencyPurge() {
+void DreamWebEngine::emergencyPurge(uint8 from) {
 	debug(2, "Ex memory: frames %d/%d, text %d/%d", _vars._exFramePos, kExframeslen, _vars._exTextPos, kExtextlen);
 
-	while (_vars._exFramePos + 4000 >= kExframeslen ||
-		   _vars._exTextPos + 400 >= kExtextlen)
+	uint16 frameBytesNeeded = 0;
+	for (int offset = 0; offset <= 1; ++offset) {
+		const Frame &freeFrame = _freeFrames._frames[3 * from + offset];
+		frameBytesNeeded += freeFrame.width * freeFrame.height;
+	}
+	const uint16 textBytesNeeded = strlen(_freeDesc.getString(from)) + 1;
+
+	while (_vars._exFramePos + frameBytesNeeded > kExframeslen ||
+		   _vars._exTextPos + textBytesNeeded > kExtextlen)
 	{
 		purgeAnItem();
 		debug(2, "Ex memory after purging: frames %d/%d, text %d/%d", _vars._exFramePos, kExframeslen, _vars._exTextPos, kExtextlen);
diff --git a/engines/dreamweb/vgagrafx.cpp b/engines/dreamweb/vgagrafx.cpp
index aa58352..2eba375 100644
--- a/engines/dreamweb/vgagrafx.cpp
+++ b/engines/dreamweb/vgagrafx.cpp
@@ -425,6 +425,7 @@ void DreamWebEngine::transferFrame(uint8 from, uint8 to, uint8 offset) {
 
 	const uint8 *src = _freeFrames.getFrameData(3*from + offset);
 	uint8 *dst = _exFrames._data + _vars._exFramePos;
+	assert(_vars._exFramePos + byteCount <= kExframeslen);
 	memcpy(dst, src, byteCount);
 
 	exFrame.setPtr(_vars._exFramePos);





More information about the Scummvm-git-logs mailing list