Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions challenge-8/submissions/aruncs/solution-template.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Package challenge8 contains the solution for Challenge 8: Chat Server with Channels.
package challenge8

import (
"errors"
"sync"
// Add any other necessary imports
)

type Message struct {
Sender string
Receiver string
Message string
}

// Client represents a connected chat client
type Client struct {
// TODO: Implement this struct
// Hint: username, message channel, mutex, disconnected flag
username string
outChannel chan Message
inChannel chan Message
connected bool
}

// Send sends a message to the client
func (c *Client) Send(message string) {
// TODO: Implement this method
// Hint: thread-safe, non-blocking send

}

// Receive returns the next message for the client (blocking)
func (c *Client) Receive() string {
// TODO: Implement this method
// Hint: read from channel, handle closed channel
message := <-c.inChannel
return message.Message
}
Comment on lines +16 to +39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Implement thread-safe Send and fix Receive channel handling.

There are several critical functional and stability issues here:

  • Send is entirely unwritten, meaning messages are never dispatched through the client's public API.
  • The Client struct lacks a mutex, leaving the connected flag vulnerable to data races.
  • outChannel is declared but never utilized.
  • Receive does not verify if the channel is closed, which can cause erratic behavior on disconnect.

Refactor the client to use a single chan string, a mutex, and a non-blocking Send method.

🛠️ Proposed refactor for Client
 // Client represents a connected chat client
 type Client struct {
-	// TODO: Implement this struct
-	// Hint: username, message channel, mutex, disconnected flag
 	username   string
-	outChannel chan Message
-	inChannel  chan Message
+	inChannel  chan string
+	mu         sync.Mutex
 	connected  bool
 }

 // Send sends a message to the client
 func (c *Client) Send(message string) {
-	// TODO: Implement this method
-	// Hint: thread-safe, non-blocking send
-
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if !c.connected {
+		return
+	}
+	select {
+	case c.inChannel <- message:
+	default:
+		// Channel full, drop message to prevent blocking
+	}
 }

 // Receive returns the next message for the client (blocking)
 func (c *Client) Receive() string {
-	// TODO: Implement this method
-	// Hint: read from channel, handle closed channel
-	message := <-c.inChannel
-	return message.Message
+	msg, ok := <-c.inChannel
+	if !ok {
+		return ""
+	}
+	return msg
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Client represents a connected chat client
type Client struct {
// TODO: Implement this struct
// Hint: username, message channel, mutex, disconnected flag
username string
outChannel chan Message
inChannel chan Message
connected bool
}
// Send sends a message to the client
func (c *Client) Send(message string) {
// TODO: Implement this method
// Hint: thread-safe, non-blocking send
}
// Receive returns the next message for the client (blocking)
func (c *Client) Receive() string {
// TODO: Implement this method
// Hint: read from channel, handle closed channel
message := <-c.inChannel
return message.Message
}
// Client represents a connected chat client
type Client struct {
username string
inChannel chan string
mu sync.Mutex
connected bool
}
// Send sends a message to the client
func (c *Client) Send(message string) {
c.mu.Lock()
defer c.mu.Unlock()
if !c.connected {
return
}
select {
case c.inChannel <- message:
default:
// Channel full, drop message to prevent blocking
}
}
// Receive returns the next message for the client (blocking)
func (c *Client) Receive() string {
msg, ok := <-c.inChannel
if !ok {
return ""
}
return msg
}


// ChatServer manages client connections and message routing
type ChatServer struct {
// TODO: Implement this struct
// Hint: clients map, mutex
mu sync.RWMutex
clients map[string]*Client
}

// NewChatServer creates a new chat server instance
func NewChatServer() *ChatServer {
// TODO: Implement this function
return &ChatServer{
clients: make(map[string]*Client, 10),
}
}

// Connect adds a new client to the chat server
func (s *ChatServer) Connect(username string) (*Client, error) {
// TODO: Implement this method
// Hint: check username, create client, add to map
s.mu.Lock()
defer s.mu.Unlock()
if s.isUsernameExists(username) {
return nil, ErrUsernameAlreadyTaken
}

client := &Client{
username: username,
inChannel: make(chan Message, 10),
outChannel: make(chan Message, 10),
connected: true,
}

s.clients[username] = client

return client, nil
}

// Disconnect removes a client from the chat server
func (s *ChatServer) Disconnect(client *Client) {
// TODO: Implement this method
// Hint: remove from map, close channels
s.mu.Lock()
defer s.mu.Unlock()

delete(s.clients, client.username)
close(client.inChannel)
close(client.outChannel)

client.connected = false
}
Comment on lines +79 to +91

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Prevent double-close panic and data races during disconnect.

If Disconnect is called twice for the same client, close(client.inChannel) will trigger a runtime panic. Additionally, to avoid data races with the Send method (which reads client.connected and writes to the channel), you must synchronize this block using the client's mutex.

🔒️ Proposed fix to synchronize and guard against panics
 func (s *ChatServer) Disconnect(client *Client) {
-	// TODO: Implement this method
-	// Hint: remove from map, close channels
 	s.mu.Lock()
 	defer s.mu.Unlock()

+	client.mu.Lock()
+	defer client.mu.Unlock()
+
+	if !client.connected {
+		return
+	}
+
 	delete(s.clients, client.username)
 	close(client.inChannel)
-	close(client.outChannel)

 	client.connected = false
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Disconnect removes a client from the chat server
func (s *ChatServer) Disconnect(client *Client) {
// TODO: Implement this method
// Hint: remove from map, close channels
s.mu.Lock()
defer s.mu.Unlock()
delete(s.clients, client.username)
close(client.inChannel)
close(client.outChannel)
client.connected = false
}
// Disconnect removes a client from the chat server
func (s *ChatServer) Disconnect(client *Client) {
s.mu.Lock()
defer s.mu.Unlock()
client.mu.Lock()
defer client.mu.Unlock()
if !client.connected {
return
}
delete(s.clients, client.username)
close(client.inChannel)
client.connected = false
}


// Broadcast sends a message to all connected clients
func (s *ChatServer) Broadcast(sender *Client, message string) {
// TODO: Implement this method
// Hint: format message, send to all clients
s.mu.RLock()
defer s.mu.RUnlock()

for _, receiver := range s.clients {
if receiver.username == sender.username {
continue
}
receiver.inChannel <- Message{
Sender: sender.username,
Receiver: receiver.username,
Message: message,
}
Comment on lines +104 to +108

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Use non-blocking Send to prevent server deadlocks.

Direct blocking channel sends while holding the server's RLock() can deadlock the entire server if a client's channel fills up (which blocks the send, thereby starving write locks like Disconnect). The shared root cause is bypassing the client's non-blocking Send method.

  • challenge-8/submissions/aruncs/solution-template.go#L104-L108: replace the blocking Message send with a call to the non-blocking receiver.Send(message).
  • challenge-8/submissions/aruncs/solution-template.go#L130-L134: replace the blocking Message send with a call to the non-blocking receiver.Send(message).
📍 Affects 1 file
  • challenge-8/submissions/aruncs/solution-template.go#L104-L108 (this comment)
  • challenge-8/submissions/aruncs/solution-template.go#L130-L134

}

}

// PrivateMessage sends a message to a specific client
func (s *ChatServer) PrivateMessage(sender *Client, recipient string, message string) error {
// TODO: Implement this method
// Hint: find recipient, check errors, send message
s.mu.RLock()
defer s.mu.RUnlock()

if !sender.connected {
return ErrClientDisconnected
}

receiver, exists := s.clients[recipient]

if !exists {
return ErrRecipientNotFound
}

receiver.inChannel <- Message{
Sender: sender.username,
Receiver: recipient,
Message: message,
}
return nil
}

func (s *ChatServer) isUsernameExists(username string) bool {
_, exists := s.clients[username]
return exists
}

// Common errors that can be returned by the Chat Server
var (
ErrUsernameAlreadyTaken = errors.New("username already taken")
ErrRecipientNotFound = errors.New("recipient not found")
ErrClientDisconnected = errors.New("client disconnected")
// Add more error types as needed
)
Loading