From 2481ef88db83fdeabb5412c48a576e0e33e13b50 Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:00:32 +0200 Subject: [PATCH 01/11] Dub - Upgraded `libsnooze` to version `1.2.0-beta` --- dub.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dub.json b/dub.json index b7a3312..4973ba2 100644 --- a/dub.json +++ b/dub.json @@ -7,7 +7,7 @@ "dependencies": { "dlog": ">=0.3.19", "eventy": ">=0.4.0", - "libsnooze": ">=1.0.0-beta" + "libsnooze": ">=1.2.0-beta" }, "description": "A sane IRC framework for the D language", "license": "LGPL-3.0", From 125bb613a7c7fbbd2fb78044c104abce8a020b2f Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:01:52 +0200 Subject: [PATCH 02/11] Receiver - Removed `hasEnsured` (commented-out already) - Call `ensure(this)` in constructor - Removed TODOs relating to ensurance Sender - Removed `hasEnsured` (commented-out already) - Call `ensure(this)` in constructor - Removed TODOs relating to ensurance --- source/birchwood/client/receiver.d | 27 +++------------------------ source/birchwood/client/sender.d | 28 +--------------------------- 2 files changed, 4 insertions(+), 51 deletions(-) diff --git a/source/birchwood/client/receiver.d b/source/birchwood/client/receiver.d index a847660..80d63ff 100644 --- a/source/birchwood/client/receiver.d +++ b/source/birchwood/client/receiver.d @@ -48,7 +48,6 @@ public final class ReceiverThread : Thread * to be processed and received */ private Event receiveEvent; - // private bool hasEnsured; /** * The associated IRC client @@ -67,7 +66,8 @@ public final class ReceiverThread : Thread super(&recvHandlerFunc); this.client = client; this.receiveEvent = new Event(); // TODO: Catch any libsnooze error here - this.recvQueueLock = new Mutex(); + this.recvQueueLock = new Mutex(); + this.receiveEvent.ensure(this); } // TODO: Rename to `receiveQ` @@ -89,10 +89,6 @@ public final class ReceiverThread : Thread /* Unlock queue */ recvQueueLock.unlock(); - // TODO: Add a "register" function which can initialize pipes - // ... without needing a wait, we'd need a ready flag though - // ... for receiver's thread start - /** * Wake up all threads waiting on this event * (if any, and if so it would only be the receiver) @@ -116,21 +112,6 @@ public final class ReceiverThread : Thread { // TODO: We could look at libsnooze wait starvation or mutex racing (future thought) - - // // Do a once-off call to `ensure()` here which then only runs once and - // // ... sets a `ready` flag for the Client to spin on. This ensures that - // // ... when the first received messages will be able to cause a wait - // // ... to immediately unblock rather than letting wait() register itself - // // ... and then require another receiveQ call to wake it up and process - // // ... the initial n messages + m new ones resulting in the second call - // if(hasEnsured == false) - // { - // receiveEvent.ensure(); - // hasEnsured = true; - // } - - // TODO: See above notes about libsnooze behaviour due - // ... to usage in our context try { receiveEvent.wait(); @@ -152,9 +133,7 @@ public final class ReceiverThread : Thread } continue; } - - - + /* Lock the receieve queue */ recvQueueLock.lock(); diff --git a/source/birchwood/client/sender.d b/source/birchwood/client/sender.d index 0de1df0..d60f8b0 100644 --- a/source/birchwood/client/sender.d +++ b/source/birchwood/client/sender.d @@ -40,7 +40,6 @@ public final class SenderThread : Thread * to be processed and sent */ private Event sendEvent; - // private bool hasEnsured; /** * The associated IRC client @@ -60,6 +59,7 @@ public final class SenderThread : Thread this.client = client; this.sendEvent = new Event(); // TODO: Catch any libsnooze error here this.sendQueueLock = new Mutex(); + this.sendEvent.ensure(this); } // TODO: Rename to `sendQ` @@ -81,10 +81,6 @@ public final class SenderThread : Thread /* Unlock queue */ sendQueueLock.unlock(); - // TODO: Add a "register" function which can initialize pipes - // ... without needing a wait, we'd need a ready flag though - // ... for sender's thread start - /** * Wake up all threads waiting on this event * (if any, and if so it would only be the sender) @@ -92,7 +88,6 @@ public final class SenderThread : Thread sendEvent.notifyAll(); } - /** * The send queue worker function */ @@ -100,24 +95,10 @@ public final class SenderThread : Thread { while(client.running) { - // // Do a once-off call to `ensure()` here which then only runs once and - // // ... sets a `ready` flag for the Client to spin on. This ensures that - // // ... when the first sent messages will be able to cause a wait - // // ... to immediately unblock rather than letting wait() register itself - // // ... and then require another sendQ call to wake it up and process - // // ... the initial n messages + m new ones resulting in the second call - // if(hasEnsured == false) - // { - // sendEvent.ensure(); - // hasEnsured = true; - // } - // TODO: We could look at libsnooze wait starvation or mutex racing (future thought) /* TODO: handle normal messages (xCount with fakeLagInBetween) */ - // TODO: See above notes about libsnooze behaviour due - // ... to usage in our context try { sendEvent.wait(); @@ -141,13 +122,6 @@ public final class SenderThread : Thread } - - - // TODO: After the above call have a once-off call to `ensure()` here - // ... which then only runs once and sets a `ready` flag for the Client - // ... to spin on - - /* Lock queue */ sendQueueLock.lock(); From ab09852fa2f7040a048b7104a709ff65703df55d Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:03:23 +0200 Subject: [PATCH 03/11] README - Upgraded required libsnooze version - Typo fixes --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index dec93a2..dd275f4 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,9 @@ dub add birchwood Birchwood depends on the following D libraries: -* `libsnooze` (atleast 1.0.0-beta) -* `eventy` (atleast 0.4.0) -* `dlog` (atleast 0.3.19) +* `libsnooze` (at least 1.2.0-beta) +* `eventy` (at least 0.4.0) +* `dlog` (at least 0.3.19) ## Usage From 5488e7902cc841e88063ac311cbc1c4373fb8651 Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:05:36 +0200 Subject: [PATCH 04/11] Receiver - Removed irrelevant TODO Sender - Removed irrelevant TODO --- source/birchwood/client/receiver.d | 1 - source/birchwood/client/sender.d | 1 - 2 files changed, 2 deletions(-) diff --git a/source/birchwood/client/receiver.d b/source/birchwood/client/receiver.d index a847660..ce106be 100644 --- a/source/birchwood/client/receiver.d +++ b/source/birchwood/client/receiver.d @@ -70,7 +70,6 @@ public final class ReceiverThread : Thread this.recvQueueLock = new Mutex(); } - // TODO: Rename to `receiveQ` /** * Enqueues the raw message into the receieve queue * for eventual processing diff --git a/source/birchwood/client/sender.d b/source/birchwood/client/sender.d index 0de1df0..98769f6 100644 --- a/source/birchwood/client/sender.d +++ b/source/birchwood/client/sender.d @@ -62,7 +62,6 @@ public final class SenderThread : Thread this.sendQueueLock = new Mutex(); } - // TODO: Rename to `sendQ` /** * Enqueues the raw message into the send queue * for eventual sending From 9d0a2bc3ce73971d1f9b9fc6b56c07cc9e80a7e9 Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:11:59 +0200 Subject: [PATCH 05/11] Client - Documented that `initEvents()` may throw an `EventyException` ErrorType - Added new member `INTERNAL_FAILURE` which could occur from errors with `Eventy` when setting up the signal handlers and event types --- source/birchwood/client/exceptions.d | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/birchwood/client/exceptions.d b/source/birchwood/client/exceptions.d index 7258c38..5564f3d 100644 --- a/source/birchwood/client/exceptions.d +++ b/source/birchwood/client/exceptions.d @@ -15,6 +15,13 @@ import std.conv : to; */ public enum ErrorType { + /** + * This could occur from errors with `Eventy` + * when setting up the signal handlers and + * event types + */ + INTERNAL_FAILURE, + /** * If the provided connection information * is invalid, such as incorrect hostname, From 42f63b9e0e5852d4029c25519bd6b2e091339f0c Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:12:24 +0200 Subject: [PATCH 06/11] Client - We now catch `EventyException` when calling `initEvents()` from within `connect()` and throw a `BirchwoodException` --- source/birchwood/client/client.d | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index 55e7453..9921431 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -10,7 +10,7 @@ import std.container.slist : SList; import core.sync.mutex : Mutex; import core.thread : Thread, dur; import std.string; -import eventy : EventyEvent = Event, Engine, EventType, Signal; +import eventy : EventyEvent = Event, Engine, EventType, Signal, EventyException; import birchwood.config; import birchwood.client.exceptions : BirchwoodException, ErrorType; import birchwood.protocol.messages : Message, encodeMessage, decodeMessage, isValidText; @@ -649,8 +649,12 @@ public class Client : Thread } /** - * Initialize the event handlers - */ + * Initialize the event handlers + * + * Throws: + * `EventyException` on error registering + * the signals and event types + */ private void initEvents() { /* TODO: For now we just register one signal type for all messages */ @@ -775,7 +779,8 @@ public class Client : Thread * Connects to the server * * Throws: - * BirchwoodException if there is an error connecting + * `BirchwoodException` if there is an error connecting + * or something failed internally */ public void connect() { @@ -820,6 +825,10 @@ public class Client : Thread { throw new BirchwoodException(ErrorType.CONNECT_ERROR); } + catch(EventyException e) + { + throw new BirchwoodException(ErrorType.INTERNAL_FAILURE, e.toString()); + } } // TODO: Do actual liveliness check here else From fe9bf31ad5edc9b59e5ac9e26b64ca8a6342927a Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:13:45 +0200 Subject: [PATCH 07/11] Client - Removed now-completed comments and commented-out-code --- source/birchwood/client/client.d | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index 9921431..2eade5d 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -798,22 +798,12 @@ public class Client : Thread /* Register default handler */ initEvents(); - // /** - // * Initialize the ready events for both the - // * receive and send queue managers, then after - // * doing so start both managers and spin for - // * both of them to enter a ready state (i.e. - // * they have ensured a waiting-pipe pair for - // * libsnooze exists) - // */ - /* Set the running status to true */ running = true; /* Start the receive queue and send queue managers */ this.receiver.start(); this.sender.start(); - // while(!receiver.isReady() || !sender.isReady()) {} /* Start the socket read-decode loop */ this.start(); From b79a3ad8ee37864211f2b81fc9c2717ecf5a9382 Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:30:53 +0200 Subject: [PATCH 08/11] Client - Now catches `SnoozeError` if a the libsnooze `Event`'s failed to have their `ensure()` call succeed ErrorType - Updated description for enum member `INTERNAL_FAILURE` Sender - Removed now-completed TODO Receiver - Removed now-completed TODO --- source/birchwood/client/client.d | 6 ++++++ source/birchwood/client/exceptions.d | 5 ++++- source/birchwood/client/receiver.d | 5 ++++- source/birchwood/client/sender.d | 5 ++++- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index 2eade5d..e06f5f3 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -19,6 +19,8 @@ import birchwood.client.receiver : ReceiverThread; import birchwood.client.sender : SenderThread; import birchwood.client.events; +import libsnooze.exceptions : SnoozeError; + import dlog; package __gshared Logger logger; @@ -819,6 +821,10 @@ public class Client : Thread { throw new BirchwoodException(ErrorType.INTERNAL_FAILURE, e.toString()); } + catch(SnoozeError e) + { + throw new BirchwoodException(ErrorType.INTERNAL_FAILURE, e.toString()); + } } // TODO: Do actual liveliness check here else diff --git a/source/birchwood/client/exceptions.d b/source/birchwood/client/exceptions.d index 5564f3d..36b7456 100644 --- a/source/birchwood/client/exceptions.d +++ b/source/birchwood/client/exceptions.d @@ -18,7 +18,10 @@ public enum ErrorType /** * This could occur from errors with `Eventy` * when setting up the signal handlers and - * event types + * event types. It can also occur if `libsnooze` + * has an error which would occur when calling + * `ensure(Thread)` for the `Receiver` and `Sender` + * threads */ INTERNAL_FAILURE, diff --git a/source/birchwood/client/receiver.d b/source/birchwood/client/receiver.d index cb1727f..a0bafa2 100644 --- a/source/birchwood/client/receiver.d +++ b/source/birchwood/client/receiver.d @@ -60,12 +60,15 @@ public final class ReceiverThread : Thread * * Params: * client = the Client to associate with + * Throws: + * `SnoozeError` on failure to construct an + * `Event` or ensure ourselves */ this(Client client) { super(&recvHandlerFunc); this.client = client; - this.receiveEvent = new Event(); // TODO: Catch any libsnooze error here + this.receiveEvent = new Event(); this.recvQueueLock = new Mutex(); this.receiveEvent.ensure(this); } diff --git a/source/birchwood/client/sender.d b/source/birchwood/client/sender.d index 4f90b23..2af5a1e 100644 --- a/source/birchwood/client/sender.d +++ b/source/birchwood/client/sender.d @@ -52,12 +52,15 @@ public final class SenderThread : Thread * * Params: * client = the Client to associate with + * Throws: + * `SnoozeError` on failure to construct an + * `Event` or ensure ourselves */ this(Client client) { super(&sendHandlerFunc); this.client = client; - this.sendEvent = new Event(); // TODO: Catch any libsnooze error here + this.sendEvent = new Event(); this.sendQueueLock = new Mutex(); this.sendEvent.ensure(this); } From 9b81573eb7ffc74c0fee84266356b0b2f9d6410e Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Sun, 25 Jun 2023 17:36:46 +0200 Subject: [PATCH 09/11] Client - Added or improved `Throws` documentation on many methods --- source/birchwood/client/client.d | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index 9921431..9948794 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -209,7 +209,7 @@ public class Client : Thread * Params: * nickname = the nickname to request * Throws: - * BirchwoodException on invalid nickname + * `BirchwoodException` on invalid nickname */ public void nick(string nickname) { @@ -244,7 +244,7 @@ public class Client : Thread * Params: * channel = the channel to join * Throws: - * BirchwoodException on invalid channel name + * `BirchwoodException` on invalid channel name */ public void joinChannel(string channel) { @@ -275,7 +275,8 @@ public class Client : Thread * Params: * channels = the channels to join * Throws: - * BirchwoodException on invalid channel name + * `BirchwoodException` on invalid channel name or + * if the list is empty */ public void joinChannel(string[] channels) { @@ -344,7 +345,8 @@ public class Client : Thread * Params: * channels = the list of channels to part from * Throws: - * BirchwoodException if the channels list is empty + * `BirchwoodException` if the channels list is empty + * or there are illegal characters present */ public void leaveChannel(string[] channels) { @@ -431,7 +433,8 @@ public class Client : Thread * message = The message to send * recipients = The receipients of the message * Throws: - * BirchwoodException if the recipients list is empty + * `BirchwoodException` if the recipients list is empty + * or illegal characters are present */ public void directMessage(string message, string[] recipients) { @@ -504,6 +507,9 @@ public class Client : Thread * Params: * message = The message to send * recipients = The receipient of the message + * Throws: + * `BirchwoodException` if the receipient's nickname + * is invalid or there are illegal characters present */ public void directMessage(string message, string recipient) { @@ -537,7 +543,7 @@ public class Client : Thread * message = The message to send * recipients = The receipients of the message * Throws: - * BirchwoodException if the channels list is empty + * `BirchwoodException` if the channels list is empty */ public void channelMessage(string message, string[] channels) { @@ -610,6 +616,9 @@ public class Client : Thread * Params: * message = The message to send * channel = The channel to send the message to + * Throws: + * `BirchwoodException` if the message or channel name + * contains illegal characters */ public void channelMessage(string message, string channel) { @@ -892,11 +901,11 @@ public class Client : Thread * Sends a message to the server by enqueuing it on * the client-side send queue. * - * A BirchwoodException is thrown if the messages - * final length exceeds 512 bytes - * * Params: * message = the message to send + * Throws: + * `BirchwoodException` if the message's length + * exceeds 512 bytes */ private void sendMessage(Message message) { From 84785529df70dbe803aa4cdf03cd9a0c9070003f Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Wed, 28 Jun 2023 08:31:41 +0200 Subject: [PATCH 10/11] Client - `leaveChannel(string)` now ensures that only valid characters are present in the provided channel's name, else throws a `BirchwoodException` --- source/birchwood/client/client.d | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index 112d957..b40e36d 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -418,14 +418,24 @@ public class Client : Thread * * Params: * channel = the channel to leave + * Throws: + * `BirchwoodException` if the channel name + * is invalid */ public void leaveChannel(string channel) { - // TODO: Add check for valid and non-empty channel names - - /* Leave the channel */ - Message leaveMessage = new Message("", "PART", channel); - sendMessage(leaveMessage); + /* Ensure the channel name contains only valid characters */ + if(isValidText(channel)) + { + /* Leave the channel */ + Message leaveMessage = new Message("", "PART", channel); + sendMessage(leaveMessage); + } + /* If invalid characters were present */ + else + { + throw new BirchwoodException(ErrorType.ILLEGAL_CHARACTERS); + } } /** From 3aa14492e23d178531c36fdd75a8f82f658ff9b4 Mon Sep 17 00:00:00 2001 From: "Tristan B. Velloza Kildaire" Date: Wed, 28 Jun 2023 08:32:44 +0200 Subject: [PATCH 11/11] Client - Removed now-completed TODOs --- source/birchwood/client/client.d | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d index b40e36d..a317369 100644 --- a/source/birchwood/client/client.d +++ b/source/birchwood/client/client.d @@ -646,13 +646,11 @@ public class Client : Thread } else { - //TODO: Invalid channel name throw new BirchwoodException(ErrorType.INVALID_CHANNEL_NAME); } } else { - //TODO: Illegal characters throw new BirchwoodException(ErrorType.ILLEGAL_CHARACTERS); } }