Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service hangs after pressing ctrl+C on the client side #255

Open
ywangwxd opened this issue Dec 3, 2024 · 9 comments
Open

service hangs after pressing ctrl+C on the client side #255

ywangwxd opened this issue Dec 3, 2024 · 9 comments
Labels
bug Something isn't working
Milestone

Comments

@ywangwxd
Copy link

ywangwxd commented Dec 3, 2024

How to reproduce:
1, start the server side
2, start the client side
3, press ctrl+C on the client side and restart the client side
Then the server side will hang. The client side will have no output and never ends by itself.

Btw, when a ctrl=C is pressed, an incomplete base64 string may be received on the server side,
so I catch the exception and return a dummy array data, otherwise there will error message like this:

File "/home/repo/diart/src/diart/utils.py", line 67, in decode_audio
byte_samples = base64.decodebytes(data.encode("utf-8"))
File "/home/miniforge3/envs/diart-spk/lib/python3.10/base64.py", line 562, in decodebytes
return binascii.a2b_base64(s)

Here is the code where I catch the exception and return the dummy data.

def decode_audio(data: Text) -> np.ndarray:
    # Decode chunk encoded in base64
    try:
        byte_samples = base64.decodebytes(data.encode("utf-8"))
        # Recover array from bytes
        samples = np.frombuffer(byte_samples, dtype=np.float32)
        return samples.reshape(1, -1)
    except Exception:
        print("Invalid base64 data")
        return np.array([[0,0]])

The server side will still hang.

@ywangwxd
Copy link
Author

ywangwxd commented Dec 3, 2024

I have made another test, defining a dummy pipeline. The dummy pipeline is copied from the transcription pipeline.
They only differ in the call() function, where a simple sample string is output for each chunk. It is interesting that this
dummy pipeline works fine even if I use ctrl+C to terminate the client side and restart it.

@ywangwxd
Copy link
Author

ywangwxd commented Dec 4, 2024

Upated again.
I have done more debug test. I stil have not got the solution, but I got more details.

I have two points of findings:

  1. If the dummy pipeline does not include any model inference, e.g., returning a dummy output right before voice segmentation, the server side does not hang.
  2. If the server side hang, it will run into this piece of code
class WebSocketAudioSource(AudioSource):
...
def _on_message_received(
        self,
        client: Dict[Text, Any],
        server: WebsocketServer,
        message: AnyStr,
    ):
        # Only one client at a time is allowed
        if self.client is None or self.client["id"] != client["id"]:
            self.client = client
        # Send decoded audio to pipeline
        #if len(message) != 5336:
        #print(f"********new message received:{len(message)} ")
        print('*********start on_next')
        self.stream.on_next(utils.decode_audio(message))
        print('*************end on_next')

The on_next is always executed, but the __call__ function in the pipeline is not executed anymore.

@ywangwxd
Copy link
Author

ywangwxd commented Dec 6, 2024

I got more update via debugging. I found that whenever the client side is terminated by a Ctrl+C, the on_next will not go into the actuall pipline defined. The status of the Subject() object is completed so on_next does nothing.

@ywangwxd
Copy link
Author

I know the reason now. When ctrl+C is pressed on the client side, the server side cannot send message to client. This will cause exception, but this excepion will change the internal status of stream, so the next client connection is not working as expected.
I can fix this by catching the exception or not sending back message to the client.

@juanmc2005
Copy link
Owner

@ywangwxd nice catch! Do you mind opening a PR with the fix? Basically, the server shouldn't hang, it should end in an error (because only 1 client is allowed for now)

@juanmc2005 juanmc2005 added the bug Something isn't working label Dec 13, 2024
@juanmc2005 juanmc2005 added this to the Version 0.9.2 milestone Dec 13, 2024
@ywangwxd
Copy link
Author

ywangwxd commented Dec 16, 2024

@ywangwxd nice catch! Do you mind opening a PR with the fix? Basically, the server shouldn't hang, it should end in an error (because only 1 client is allowed for now)

Maybe it is imprecise to say it hangs, it is just not working as expected. The on_next function is stilled called, but the pipeline is not executed. I am not familiar with the internal state of the stream. It seems that the stream is already dead internally but did not notify outside users. What I found is actually not the problem of the server side. We can only say the server side has not handled any exceptions from the client side robustly. If the server side does not rely on the client side so tightly, it is okay.

@ywangwxd
Copy link
Author

@ywangwxd nice catch! Do you mind opening a PR with the fix? Basically, the server shouldn't hang, it should end in an error (because only 1 client is allowed for now)

My code is ugly for a PR.

@juanmc2005
Copy link
Owner

The errors are not explicitly sent from the client to the server, this is probably why it seems to hang. However, I don't understand why on_next would keep getting called in the server as it's not receiving audio anymore.

@ywangwxd
Copy link
Author

ywangwxd commented Dec 18, 2024

The errors are not explicitly sent from the client to the server, this is probably why it seems to hang. However, I don't understand why on_next would keep getting called in the server as it's not receiving audio anymore.

Yeah, the serve should throw an exception or at least reset its status to allow a new client connection. The lattter is reasonable because in the reality there may be various reasons a client can lost its connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants