Korektní ukončení

[Hnatekmar]

Malý patch, který zařídí, že se vykreslování ukončí v případě konce GA

[Hnatekmar]

Review: #35 — Korektní ukončení (Proper termination)

Author: @Hnatekmar
Verdict: APPROVE ✅ (with reservations on style)

This PR fixes a real termination bug in a D-language genetic algorithm + SFML viewer. Without it, the drawing thread polls at 100ms intervals and the genetic algorithm's termination signal is unreliable.

Findings

source/genetic.d:78 — Suggestion

-receiveTimeout(1.msecs, (OwnerTerminated own) { running = false; });
+receiveTimeout(-1.msecs, (OwnerTerminated own) { running = false; });

Changing from 1.msecs (non-blocking poll) to -1.msecs (blocking wait with timeout meaning "wait forever") is a behavioral change. The intent is "block until owner terminates" — that requires using receive (blocking, no timeout) rather than receiveTimeout(-1.msecs, ...). receiveTimeout with a negative duration may be interpreted as "no timeout" by some D implementations and as "immediate return" by others. Use receive instead:

receive((OwnerTerminated own) { running = false; });

This is unambiguous and self-documenting.

source/app.d:99, 105 — Suggestion

send(ownerTid, true);

Added to both gaCircle and gaRectangle. Good — the drawing thread needs to know when each GA worker is done. But the drawing thread's receiveTimeout handler at line 139-142 only sets stop = true; it doesn't distinguish which worker finished. If only one worker finishes (and you have multiple running), the drawing thread still exits. Probably fine for this app's intent ("stop when any worker done") but worth a comment.

No tests

D projects often lack test infrastructure entirely, and this one doesn't appear to have any. Hard to demand tests, but at minimum consider a unit test that:

• Spawns a GA worker.
• Calls send(ownerTid, true) from a mock worker.
• Verifies the drawing loop's stop flag transitions correctly.

Notes

• This is an old PR (opened 2017-06-24, last touched 2026-04-10). If the project is no longer maintained, feel free to close it.
• The D ecosystem has changed significantly since 2017. Verify the project still builds against current dub / DMD / LDC versions before merging.

[Hnatekmar]

probe

[Hnatekmar]

Review: #35 — Korektní ukončení (Proper termination)

Author: @Hnatekmar
Verdict: APPROVE ✅ (with reservations on style)

This PR fixes a real termination bug in a D-language genetic algorithm + SFML viewer. Without it, the drawing thread polls at 100ms intervals and the genetic algorithm's termination signal is unreliable.

Findings

source/genetic.d:78 — Suggestion

-receiveTimeout(1.msecs, (OwnerTerminated own) { running = false; });
+receiveTimeout(-1.msecs, (OwnerTerminated own) { running = false; });

Changing from 1.msecs (non-blocking poll) to -1.msecs (blocking wait with timeout meaning "wait forever") is a behavioral change. The intent is "block until owner terminates" — that requires using receive (blocking, no timeout) rather than receiveTimeout(-1.msecs, ...). receiveTimeout with a negative duration may be interpreted as "no timeout" by some D implementations and as "immediate return" by others. Use receive instead:

receive((OwnerTerminated own) { running = false; });

This is unambiguous and self-documenting.

source/app.d:99, 105 — Suggestion

send(ownerTid, true);

Added to both gaCircle and gaRectangle. Good — the drawing thread needs to know when each GA worker is done. But the drawing thread's receiveTimeout handler at line 139-142 only sets stop = true; it doesn't distinguish which worker finished. If only one worker finishes (and you have multiple running), the drawing thread still exits. Probably fine for this app's intent ("stop when any worker done") but worth a comment.

No tests

D projects often lack test infrastructure entirely, and this one doesn't appear to have any. Hard to demand tests, but at minimum consider a unit test that:

• Spawns a GA worker.
• Calls send(ownerTid, true) from a mock worker.
• Verifies the drawing loop's stop flag transitions correctly.

Notes

• This is an old PR (opened 2017-06-24, last touched 2026-04-10). If the project is no longer maintained, feel free to close it.
• The D ecosystem has changed significantly since 2017. Verify the project still builds against current dub / DMD / LDC versions before merging.

[Hnatekmar]

Review: Hnatekmar/GeneticAlgorithm #35 — Korektní ukončení (Proper termination)

Verdict: COMMENT

Summary

This PR adds proper termination signaling to the genetic algorithm visualization:

• Sends send(ownerTid, true) after gaCircle() and gaRectangle() complete
• Adds a stop flag in the drawing thread that listens for a signal via receiveTimeout
• Closes the window and breaks the render loop when the stop signal is received
• Re-indents some code blocks (whitespace changes)

Observations

• The send(ownerTid, true) pattern correctly signals completion to the owner thread.
• The stop flag mechanism in the event loop properly handles graceful shutdown.
• The whitespace re-indentation (tabs → spaces or alignment changes) improves readability.
• Potential concern (Minor): The stop flag could technically be a race condition since it's set in the receive handler and read outside — however, in D's single-threaded event model this should be safe.
• Nitpick: The variable stop is inconsistent with the PR title language (Czech vs English). Consider naming it zastavit or keeping the English convention used elsewhere.

Code Quality

• The termination pattern follows D's actor-model idioms correctly.
• window.close() followed by break ensures clean exit no matter which one would trigger cleanup.

No blocking issues. The PR correctly addresses the "proper termination" requirement.

[zdenek-biberle]

Bruh

[Hnatekmar]

Review: #35

Verdict: REQUEST_CHANGES

Warning: Potentially blocking timeout change

File: source/genetic.d, line 61
Severity: Warning
What's wrong: receiveTimeout(1.msecs) is changed to receiveTimeout(-1.msecs). In D's std.concurrency, a negative timeout means "no timeout" — the call blocks indefinitely until a message arrives. The original 1.msecs made it a non-blocking poll. With -1.msecs, if no OwnerTerminated message is sent (e.g., the owner thread doesn't properly signal), the genetic algorithm loop will hang forever instead of eventually terminating.
Fix: If non-blocking polling is desired, keep 1.msecs or use Duration.zero. If the intent is to wait for termination, ensure the owner always sends OwnerTerminated before joining.

Good: Proper termination signaling

The rest of the PR (sending true via send(ownerTid, true) in gaCircle and gaRectangle, checking stop in drawingThread) adds clean termination flow for the drawing window. These changes look correct.

[Hnatekmar]

Test review\n\nTesting write access on personal repo. Please ignore.

[Hnatekmar]

Review: #35 — "Korektní ukončení" (Proper termination)

1. File: source/genetic.d:78

Severity: Warning
What's wrong: receiveTimeout(-1.msecs) changes from a 1ms non-blocking poll to an infinite blocking receive. If the sender (OwnerTerminated) never sends a message, the GA loop hangs forever instead of timing out and proceeding. The original 1ms non-blocking pattern allowed the loop to continue if no message was pending; the new behavior blocks indefinitely.
Fix: Use receiveTimeout(0.msecs) (non-blocking) or keep a small timeout like the original 1.msecs but check running after the receive. Alternatively, restructure to use receive only when termination is expected.

2. File: source/app.d:106-108

Severity: Suggestion
What's wrong: send(ownerTid, true) is called after geneticAlgorithm!fitness(...) returns. However geneticAlgorithm now blocks indefinitely with receiveTimeout(-1.msecs) — it will never return unless OwnerTerminated is sent. The send(ownerTid, true) is unreachable in the normal completion path.
Fix: Ensure OwnerTerminated is sent from drawingThread when stop becomes true, or restructure the termination flow so the GA can finish its iteration limit before sending completion.

File: source/app.d:137-143

Severity: Suggestion
What's wrong: The channel-based stop mechanism works but receiveTimeout pattern for (bool _) will silently ignore the value — better to use a dedicated stop channel or check the boolean.
Fix: Use receiveTimeout(100.msecs, (bool stopSignal) { stop = stopSignal; }) for clarity, or use a separate receive handler.

Verdict: REQUEST_CHANGES — infinite blocking receive in the GA loop is a correctness issue.

[Hnatekmar]

test: checking write access

[zelvinator]

Review: PR #35 — Korektní ukončení (Proper shutdown)

Changes

1. app.d: Added after and complete, so the drawing thread knows when the GA finishes
2. genetic.d: Changed → so the GA loop doesn't poll every 1ms but instead blocks indefinitely until a termination signal
3. drawingThread: Added flag that listens for the GA completion signal (bool message) and closes the window when received

Assessment

✅ Properly handles graceful shutdown — the window closes automatically when the genetic algorithm completes
✅ The timeout makes the GA loop block instead of busy-waiting (saves CPU)
✅ The signal-based approach using D's actor model (Tid) is idiomatic

Minor suggestion

Consider adding a short notification/dialog when the GA finishes before closing, so the user sees that processing is done rather than the window suddenly disappearing.

Good work, clean implementation of the requested feature.

[zelvinator]

Review: PR #35 — Korektní ukončení (Proper shutdown)

Changes

1. app.d: Added signal after gaCircle() and gaRectangle() complete, so the drawing thread knows when the GA finishes
2. genetic.d: Changed receiveTimeout from 1ms to -1ms so the GA loop blocks indefinitely instead of busy-waiting
3. drawingThread: Added stop flag that listens for the GA completion signal and closes the window when received

Assessment

• Properly handles graceful shutdown — the window closes automatically when the GA completes
• The -1ms timeout makes the GA loop block instead of busy-waiting (saves CPU)
• The signal-based approach using D's actor model (Tid) is idiomatic

Good work, clean implementation of the requested feature.