eclipse-ee4j / eclipse-ee4j/tyrus

JdkClientContainer RemoteEndpoint.Basic.sendText(String) send blocks indefinitely

Open
#678 8 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
128
Forks
49
PR merge metrics
No merged PRs in 30d

Description

We're having Issues with the Client where a `javax.websocket.RemoteEndpoint.Basic.sendText(String)` will send indefinitely (/"deadlock"). The occurrence of this issue is very rare.

Stack trace of hanging (/blocked) thread:
```
sun.misc.Unsafe.park(Native Method)
java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836)
java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:997)
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1304)
java.util.concurrent.CountDownLatch.await(CountDownLatch.java:231)
org.glassfish.tyrus.core.TyrusFuture.get(TyrusFuture.java:53)
org.glassfish.tyrus.core.TyrusRemoteEndpoint$Basic.processFuture(TyrusRemoteEndpoint.java:143)
org.glassfish.tyrus.core.TyrusRemoteEndpoint$Basic.sendText(TyrusRemoteEndpoint.java:80)
```

The same issue occurs with `javax.websocket.RemoteEndpoint.Async.sendText(String, SendHandler)` were `SendHandler.onResult` is never called.

It seems that the request is actually really not sent.
After investigating further I recognized that the class `org.glassfish.tyrus.container.jdk.client.TaskQueueFilter` is **not** thread-safe. Or - to be precise - locked inconsistently.
As I understood the TaskQueueFilter should guarantee that only one write operation is executed at a time (no overlapping, no concurrent writes)

In case the write-method is called a write task is enqueued. If the caller was the first to enqueue a task (taskLock false -> true), processTask is called which will trigger the write to the downstreamFilter. Any subsequent write-task is enqueued; processTask is not executed (as there is already a task in progress; taskLock == true).
As the write-handler completed, the next task from the queue is processed and so on. After the queue is empty taskLock is _released_ (taskLock true-> false) so that the next write-method invocation would start described process again.

https://github.com/eclipse-ee4j/tyrus/blob/f6dfb4749019502c2030848dd48a19950543834c/containers/jdk-client/src/main/java/org/glassfish/tyrus/container/jdk/client/TaskQueueFilter.java#L53-L68

**BUT** The _release_ criteria is not checked atomic.
Assume the following Scenario
1. Thread-1 enters processTask(); `taskQueue.poll()` returns null
1. Thread-2 invokes write; offers the write-task to the taskQueue
1. Thread-2 checks if processTask() should be called to initiate task processing; `taskLock == true` so compareAndSet will return `false` and processTask **is not** invoked
1. Thread-1 `task == null` returns true and taskLock is set to `false`

Now we have a queued task which will not be processed by any thread.

It will _heal_ somehow if another thread will call write. (processTask would catch-up the already queued tasks). But in my scenario it will never _heal_ because i call write (/sendText) from a single thread.

Contributor guide

Open the contributing guide

Research direction

Start with containers/jdk-client/src/main/java/org/glassfish/tyrus/container/jdk/client/TaskQueueFilter.java, especially lines 53-68, and trace how write tasks acquire and release taskLock. Reproduce the interleaving described in the issue, then verify that a task queued during release is processed and that synchronous and asynchronous sends complete.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.