Skip to content

Commit 7eb0818

Browse files
committed
Fix race condition segfault in JSSEngineReferenceImpl cleanup
The finalizer thread was experiencing segfaults during PR.Shutdown() calls due to a race condition where multiple threads could execute cleanup operations simultaneously on the same object. Root cause: - cleanup(), closeInbound(), closeOutbound(), and tryCleanup() methods were not synchronized - Multiple threads could simultaneously check and modify the boolean flags (closed_fd, is_inbound_closed, is_outbound_closed) - One thread could close/free ssl_fd while another thread was still using it in PR.Shutdown(), causing SIGSEGV in NSS memcpy Changes: - Add synchronized modifier to closeInbound(), closeOutbound(), cleanup(), and tryCleanup() methods - The synchronized keyword provides both mutual exclusion (only one thread can execute these methods at a time) and memory visibility (changes to fields are visible to other threads) This prevents concurrent cleanup operations from corrupting the native PRFileDesc pointer and eliminates the segfault during finalization. Assisted-by: Claude Sonnet 4.5
1 parent c527e37 commit 7eb0818

1 file changed

Lines changed: 4 additions & 4 deletions

File tree

base/src/main/java/org/mozilla/jss/ssl/javax/JSSEngineReferenceImpl.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,7 @@ public void beginHandshake() throws SSLException {
736736
}
737737

738738
@Override
739-
public void closeInbound() {
739+
public synchronized void closeInbound() {
740740
debug("JSSEngine: closeInbound()");
741741

742742
if (!is_inbound_closed && ssl_fd != null && !closed_fd) {
@@ -750,7 +750,7 @@ public void closeInbound() {
750750
}
751751

752752
@Override
753-
public void closeOutbound() {
753+
public synchronized void closeOutbound() {
754754
debug("JSSEngine: closeOutbound()");
755755

756756
if (!is_outbound_closed && ssl_fd != null && !closed_fd) {
@@ -1675,7 +1675,7 @@ public SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int length, ByteBuffe
16751675
* connection.
16761676
*/
16771677
@Override
1678-
public void tryCleanup() {
1678+
public synchronized void tryCleanup() {
16791679
debug("JSSEngine: tryCleanup()");
16801680
if (is_inbound_closed && is_outbound_closed) {
16811681
// throw new RuntimeException("Probably shouldn't be here!");
@@ -1688,7 +1688,7 @@ public void tryCleanup() {
16881688
* data streams if still open.
16891689
*/
16901690
@Override
1691-
public void cleanup() {
1691+
public synchronized void cleanup() {
16921692
debug("JSSEngine: cleanup()");
16931693

16941694
if (!is_inbound_closed) {

0 commit comments

Comments
 (0)