fix: fix log producer stopping block indefinitely

[lefinal]

Fixes an issue where stopping the log producer would block indefinitely if the log producer has exited before.

What does this PR do?

Fixes stopping the log producer blocking by not requiring a read on the stopProducer channel but simply closing it. There is no synchronization for the log producer to finish running anyways, so this should be fine.

Why is it important?

If the log producer exits due to an exited container, StopLogProducer will hang indefinitely.

Related issues

Closes #1407 Closes #1669

How to test this PR

Test has been added.

Follow-ups

stopProducer is currently not safe for concurrent use. Maybe add some synchronization in the future.

[netlify[bot]]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	ca049655293fd1cfe50dc66c9564abd53a1b0147
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/654abe763e9b53000818d968
😎 Deploy Preview	https://deploy-preview-1783--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[mdelapenya]

Hey @lefinal thanks for this! Do you think it is related to https://github.com/testcontainers/testcontainers-go/pull/1777?

[lefinal]

Hey @lefinal thanks for this! Do you think it is related to #1777?

The mentioned issue regarding panics is not the same. However, the changes from #1777 will accomplish the same thing. The author just uses context.Context and context.CancelFunc instead of closing the channel. Both approaches will avoid StopLogProducer hanging.

[mdelapenya]

Hey @lefinal thanks for this! Do you think it is related to #1777?

The mentioned issue regarding panics is not the same. However, the changes from #1777 will accomplish the same thing. The author just uses context.Context and context.CancelFunc instead of closing the channel. Both approaches will avoid StopLogProducer hanging.

So, if you mind 🙏 , once we merge that one (already reviewed, tested and commented by the community), we can close this one.

[lefinal]

Hey @lefinal thanks for this! Do you think it is related to #1777?

The mentioned issue regarding panics is not the same. However, the changes from #1777 will accomplish the same thing. The author just uses context.Context and context.CancelFunc instead of closing the channel. Both approaches will avoid StopLogProducer hanging.

So, if you mind 🙏 , once we merge that one (already reviewed, tested and commented by the community), we can close this one.

Absolutely! Only wanted to provide a quick fix in case #1777 takes longer to review and merge :)

[mdelapenya]

Only wanted to provide a quick fix in case #1777 takes longer to review and merge

Would you mind resolving the conflicts if possible? 🙏 I think this fix if easier to merge that #1777

[lefinal]

Only wanted to provide a quick fix in case #1777 takes longer to review and merge

Would you mind resolving the conflicts if possible? 🙏 I think this fix if easier to merge that #1777

Done :)

[jrfeenst]

Any updates on this?

[mdelapenya]

I think https://github.com/testcontainers/testcontainers-go/pull/1971 could be related to this

[mdelapenya]

@lefinal is #1971 superseding this one? Could you please verify that? 🙏

[mdelapenya]

Hey @lefinal did you check https://github.com/testcontainers/testcontainers-go/pull/1783#issuecomment-1944534360?

Please let me know if I can close this one 🙏

Cheers!