[SnapshotHandler] Small improvements

[marcdumais-work]

What it does

I noticed a few things by inspecting the code of SnapshotHandler, that I am attempting to address in this commit:

In publish():

• a call is made to super.publish(), which calls parent class FileHandler's publish(). This seems unnecessary - if the file handler configuration is present, it could result in potentially the double-publishing of log records - in the snapshot log and in the file log, or more likely do nothing. In cases where this might be desirable, I think it can be accomplished by configuring handler hierarchy such that a FileHandler is the parent of SnapshotHandler.
• instead, call the overridden isLoggable() to check whether the log records should be logged, and if so, "publish" to the in-memory queue.

In the local isLoggable():

• remove the redundant null check, which is performed already in super.isLoggable()
• remove the seemingly strange log level check that filters-out anything higher than "FINE".

In addToSnapshot():

• update the in-memory "stack-tracking" structure: when the last entry for a given pid is removed, remove the corresponding list. Similarly, if that list was the last entry for the pid-level parent structure, remove that one as well. The lists will be recreated if needed later (using the existing computeIfAbsent()). This change might result in these lists being allocated/freed multiple times, but should prevent the case where a program, which creates many threads, ends-up with very many of these empty lists staying allocated indefinitely, each consuming a little heap memory.
• Make the draining of the in-memory queue to file atomic by having "publish()" use a new, empty queue, while the old queue is drained and then discarded. Before doing this, I think new log records might get added to the draining queue, if some logging occurred during draining.

How to test

Verify that CI passes. Ideally, I would like if there were some higher-level tests for this handler, to complement the current unit tests.

Follow-ups

N/A

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

[marcdumais-work]

I set the PR back to draft because I noticed an issue that I think needs addressing.

Before this PR, I think the LogLevel filtering and other tasks performed by isLoggable() were not effectively working/called, before a LogRecord was added to the in-memory queue. This means that all (non-null) log records were sent to addToSnapshot() and when appropriate, tracked in the global fStacks structure from that method.

Now that LogLevel filtering is working, we can end-up in the situation where we might have unmatched "B" or "E" events, for the same PID/TID, if e.g. the log level is changed dynamically, and

• the new log level is more restricted, such that some past "B" events are already being tracked, but the corresponding "E" events will be discarded when they happen and so never be tracked/matched to their "B" event.
• the new log level is less restrictive, such that the corresponding previous "B" events were discarded and not tracked, but the new "E" event will be let through, causing problems, I think?

In the first case, I think there will be some "unpaired" "B" events being tracked, that I think will be in the tracking structure forever. In the second case, the current code might try to remove a non-existing list element from the tracking structure.

More generally, I wonder if we can get into similar trouble if the logger is dynamically enabled/disabled, now that fIsEnabled is checked before calling addToSnapshot? If would seem so. Would it be necessary to reset tracking and possibly dispose of the in-memory event queue, when such impactful changes are performed in the handler?

[marcdumais-work]

More generally, I wonder if we can get into similar trouble if the logger is dynamically enabled/disabled, now that fIsEnabled is checked before calling addToSnapshot? If would seem so. Would it be necessary to reset tracking and possibly dispose of the in-memory event queue, when such impactful changes are performed in the handler?

Alternatively, it might work to perform the tracking of "B" and "E" events no matter the handler's current settings (i.e. logging is disabled, LogEvent log level too low to be loggable, ...) so that the fStacks structure is kept up-to-date and consistent, no matter what?