Browse Source

Server: avoid re-use of recent ParticleSpawner and Sound IDs (#14045)

This improves the reliability when removing and re-adding handles quickly.
Looping through the entire ID range avoids collisions caused by any race condition.
SmallJoker 6 months ago
parent
commit
a7e5456099
4 changed files with 25 additions and 16 deletions
  1. 14 6
      src/server.cpp
  2. 1 1
      src/server.h
  3. 9 9
      src/serverenvironment.cpp
  4. 1 0
      src/serverenvironment.h

+ 14 - 6
src/server.cpp

@@ -2171,12 +2171,18 @@ void Server::SendPlayerSpeed(session_t peer_id, const v3f &added_vel)
 
 inline s32 Server::nextSoundId()
 {
-	s32 ret = m_next_sound_id;
-	if (m_next_sound_id == INT32_MAX)
-		m_next_sound_id = 0; // signed overflow is undefined
-	else
-		m_next_sound_id++;
-	return ret;
+	s32 free_id = m_playing_sounds_id_last_used;
+	while (free_id == 0 || m_playing_sounds.find(free_id) != m_playing_sounds.end()) {
+		if (free_id == INT32_MAX)
+			free_id = 0; // signed overflow is undefined
+		else
+			free_id++;
+
+		if (free_id == m_playing_sounds_id_last_used)
+			return 0;
+	}
+	m_playing_sounds_id_last_used = free_id;
+	return free_id;
 }
 
 s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
@@ -2232,6 +2238,8 @@ s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
 
 	// old clients will still use this, so pick a reserved ID (-1)
 	const s32 id = ephemeral ? -1 : nextSoundId();
+	if (id == 0)
+		return 0;
 
 	float gain = params.gain * params.spec.gain;
 	NetworkPacket pkt(TOCLIENT_PLAY_SOUND, 0);

+ 1 - 1
src/server.h

@@ -695,7 +695,7 @@ private:
 		Sounds
 	*/
 	std::unordered_map<s32, ServerPlayingSound> m_playing_sounds;
-	s32 m_next_sound_id = 0; // positive values only
+	s32 m_playing_sounds_id_last_used = 0; // positive values only
 	s32 nextSoundId();
 
 	ModStorageDatabase *m_mod_storage_database = nullptr;

+ 9 - 9
src/serverenvironment.cpp

@@ -1637,16 +1637,16 @@ u32 ServerEnvironment::addParticleSpawner(float exptime)
 	// Timers with lifetime 0 do not expire
 	float time = exptime > 0.f ? exptime : PARTICLE_SPAWNER_NO_EXPIRY;
 
-	u32 id = 0;
-	for (;;) { // look for unused particlespawner id
-		id++;
-		std::unordered_map<u32, float>::iterator f = m_particle_spawners.find(id);
-		if (f == m_particle_spawners.end()) {
-			m_particle_spawners[id] = time;
-			break;
-		}
+	u32 free_id = m_particle_spawners_id_last_used;
+	while (free_id == 0 || m_particle_spawners.find(free_id) != m_particle_spawners.end()) {
+		if (free_id == m_particle_spawners_id_last_used)
+			return 0; // full
+		free_id++;
 	}
-	return id;
+
+	m_particle_spawners_id_last_used = free_id;
+	m_particle_spawners[free_id] = time;
+	return free_id;
 }
 
 u32 ServerEnvironment::addParticleSpawner(float exptime, u16 attached_id)

+ 1 - 0
src/serverenvironment.h

@@ -512,6 +512,7 @@ private:
 	// Particles
 	IntervalLimiter m_particle_management_interval;
 	std::unordered_map<u32, float> m_particle_spawners;
+	u32 m_particle_spawners_id_last_used = 0;
 	std::unordered_map<u32, u16> m_particle_spawner_attachments;
 
 	// Environment metrics