Terminal session output during shutdown (Issue 645)

[JamesWekel]

Fish,

The request handles two items:

1. replaces clean_screen() in panel_cleanup with blank_panel() to fix Issue 645.
2. adds some synchronization, controlled by hsmisc.c, between logger and panel. This synchronization improves terminal console message capture at shutdown. A new message 'HHC01421I Shutdown: %s' has been added.

Sorry, I should have separated this into two commits.

I've tested the request on my Linux system and on Windows (only very basic tests). On my Linux system with Regina Rexx, 'make check' completes.

Again, the request need your review, and maybe some testing by Bill on other OS's.

Jim

[JamesWekel]

Fish,

I'm not a fan of spin-wait loops to coordinate multiple threads and would prefer locks myself. But, this option seemed the simplest method as we are try to coordinate shutdown. I concur it is fragile but that's why there is a shutdown safety check if the coordinate fails. Also, this method does not change any cross-library dependencies (an area that I currently have no knowledge).

Before committing the pull request, I concur with Bill that this needs to be tested on a few OSs. This is not critical, so absolutely there is no rush. Safety first.

Jim

[JamesWekel]

Fish, I noticed a possible timing error ... so an updated commit.

Jim

[Fish-Git]

Fish, I noticed a possible timing error ... so an updated commit.

Thanks, James. I pulled your changes, rebuilt and retested, and everything looks fine to me.

So I'm inclined to accept your Pull Request as-is (even with my previously mentioned concerns), but of course am not going to officially approve it just yet,

I want to wait until both Bill and Enrico have completed their testing first. Once their testing is complete and it looks okay to them, only then will I re-review and officially approve and Merge your Pull Request.

Fair?

[Fish-Git]

I want to wait until both Bill and Enrico have completed their testing first. Once their testing is complete and it looks okay to them, only then will I re-review and officially approve and Merge your Pull Request.

Bill? Enrico? Any progress on this yet? I'd like to get this fix merged.

[JamesWekel]

No rush on my end. Always prefer some testing/review other than me!

Jim

[Fish-Git]

No rush on my end. Always prefer some testing/review other than me!

Agreed! But given that the patch does seem to resolve the issue, and no problems have been reported against it (and the fact that it does fix a known problem after all), I am naturally inclined to want to get the fix merged into our base as soon as possible.

And given that the problem is purely cosmetic anyhow, I am inclined to Merge it anyway, even without it being thoroughly tested. If you guys suddenly find a problem with it at some point during your testing, we can always address it then. But I fail to see the need to delay the fix's implementation simply because it hasn't been fully tested on every variant of Linux under the sun!

I know you guys are busy working on other parts of Hercules just as I am, and that some of you do have a personal life (whatever the heck that is) and personal life issues that takes priority over Hercules (to which I am very understanding and sympathetic to!), at the same time I do not feel it should delay implementation of what appears is a good fix.

Beside, as I said, there's nothing stopping you from continuing to test the change even after it's been officially implemented, so I am very inclined to accept this change and Merge is anyway even without it being "thoroughly" test (i.e. only lightly tested).

Does anyone have an objection to that?

[wrljet]

Yes, just put it in.

[JamesWekel]

No objection from me, but it is my pull request so I might be slightly biased  :-)

[Juergen-Git]

Hi Fish

No objection, just go ahead. I’m currently a “heavy user” of shutdowns and I can confirm that it works flawlessly. 😊

Cheers Jürgen

[Fish-Git]

Merged!