Skip to content

Commit

Permalink
Fix(dcsctp): Fix memory leak. (#2182)
Browse files Browse the repository at this point in the history
There were reference cycles through JNI, which the garbage collector can't detect.
  • Loading branch information
JonathanLennox authored Jun 21, 2024
1 parent af2ff24 commit 793df5a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
2 changes: 1 addition & 1 deletion jvb/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>jitsi-dcsctp</artifactId>
<version>1.0-2-g2d8eee4</version>
<version>1.0-3-gaf9d564</version>
</dependency>
<!-- https://mvnrepository.com/artifact/org.slf4j/slf4j-api -->
<!-- we inherit an old version of slf4j-api from tinder, which pcap4j doesn't work with. Adding slf4j-api 1.7.30 (the current stable) as a dep of jvb fixes the problem. -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.jitsi.utils.logging2.Logger
import org.jitsi.utils.logging2.createChildLogger
import org.jitsi.videobridge.sctp.SctpConfig
import org.jitsi.videobridge.util.TaskPools
import java.lang.ref.WeakReference
import java.time.Clock
import java.time.Instant
import java.util.concurrent.Future
Expand Down Expand Up @@ -109,7 +110,7 @@ abstract class DcSctpBaseCallbacks(
) : DcSctpSocketCallbacks {
/* Methods we can usefully implement for every JVB socket */
override fun createTimeout(p0: DcSctpSocketCallbacks.DelayPrecision): Timeout {
return ATimeout()
return ATimeout(transport)
}

override fun Now(): Instant {
Expand Down Expand Up @@ -142,7 +143,11 @@ abstract class DcSctpBaseCallbacks(
transport.logger.info("Surprising SCTP callback: incoming streams ${incomingStreams.joinToString()} reset")
}

private inner class ATimeout : Timeout {
private class ATimeout(transport: DcSctpTransport) : Timeout {
// This holds a weak reference to the transport, to break JNI reference cycles
private val weakTransport = WeakReference(transport)
private val transport: DcSctpTransport?
get() = weakTransport.get()
private var timeoutId: Long = 0
private var scheduledFuture: ScheduledFuture<*>? = null
private var future: Future<*>? = null
Expand All @@ -152,11 +157,11 @@ abstract class DcSctpBaseCallbacks(
scheduledFuture = TaskPools.SCHEDULED_POOL.schedule({
/* Execute it on the IO_POOL, because a timer may trigger sending new SCTP packets. */
future = TaskPools.IO_POOL.submit {
transport.socket.handleTimeout(timeoutId)
transport?.socket?.handleTimeout(timeoutId)
}
}, duration, TimeUnit.MILLISECONDS)
} catch (e: Throwable) {
transport.logger.warn("Exception scheduling DCSCTP timeout", e)
transport?.logger?.warn("Exception scheduling DCSCTP timeout", e)
}
}

Expand Down

0 comments on commit 793df5a

Please sign in to comment.