Use analysis abort in server mode

[karoliineh]

This branch has the initial abort logic, relating to #20. Although the initial version works for aborting the analysis and starting a new one, it executes the code in GobPie following the reanalysis even after the abort. This results in the actions (reading the results from the socket and printing "analysis finished") being executed consecutively as many times as the analysis was restarted.

To solve this issue, we have discussed with @sim642 to try and use Java threads for the actions from triggering analyses to requesting results.

[sim642]

Actually, the solution might be a lot simpler than threads and interrupting. If Goblint receives the aborting signal, then it still replies with a message in the socket, but the difference is that it should contain "status": "Aborted". So the GobPie code that's waiting for a response will get one (and it actually must read it, otherwise the IDs probably get mixed up), but can then decide not to separately request messages and quietly return.

[karoliineh]

I implemented the solution, where the result from the response is checked and reanalyze method returns in case of an abort (instead of trying to read messages and getting stuck because of it).

Although, now if the analysis is aborted, a pop-up with "GobPie finished analyzing the code." is still shown. Unfortunately, I could not find a way how to turn it off, because it seems that MagpieBridge creates this message each time the analyze method finishes, even if any results have not been given to be consumed by the server (MagpieServer.java):

if (a != null) {
      this.forwardMessageToClient(
          new MessageParams(MessageType.Info, a.source() + " started analyzing the code."));
      a.analyze(fileManager.getSourceFileModules().values(), this, rerun);
      this.forwardMessageToClient(
          new MessageParams(MessageType.Info, a.source() + " finished analyzing the code."));
    }

which is a bit odd behaviour, because we do not really finish the analysis with abort (no results can be expected to be shown).

[sim642]

One incredibly stupid way to prevent the "finished analyzing the code" message from being shown would be to have Thread.sleep(Long.MAX_VALUE); before our implementation of analyze returns (if the read status was aborted). Although that might have some bad effect by leaving a large number of threads around waiting (practically) forever.

[michael-schwarz]

This sounds like exactly the sort of thing where one might want to file a bug report / do a PR upstream, doesn't it?

On Tue, May 31, 2022, 7:12 PM Simmo Saan @.***> wrote:

One incredibly stupid way to prevent the "finished analyzing the code" message from being shown would be to have Thread.sleep(Long.MAX_VALUE); before our implementation of analyze (if the read status was aborted). Although that might have some bad effect by leaving a large number of threads around waiting (practically) forever.

— Reply to this email directly, view it on GitHub https://github.com/goblint/GobPie/pull/34#issuecomment-1142400337, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADJME3MXCGNFUH5ROSUG6F3VMZCBDANCNFSM5XEQCIGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[sim642]

This sounds like exactly the sort of thing where one might want to file a bug report / do a PR upstream, doesn't it?

Sure! An upstream solution probably needs to be a bit more thought through though since there might be more than two possible results an analyzer might want to return. But an upstream issue definitely wouldn't hurt.

[karoliineh]

I realized that the analyze method was indeed itself blocking, and it had nothing to do with Magpie's threading #35.

I now run the process of communicating with Goblint in a separate thread, so that the analyze method itself will finish right away and not block the next call. When there is another call for analyze (triggered by saving a file), and the previous analysis is running, it sends the SIGINT to Goblint and kills the process that is running in the separate thread.

With this the pop-up situation:

Although, now if the analysis is aborted, a pop-up with "GobPie finished analyzing the code." is still shown. ... which is a bit odd behaviour, because we do not really finish the analysis with abort (no results can be expected to be shown).

is even worse, because the analyze method finishes instantly and the pop-up will say "GobPie finished analyzing the code.", although the analysis is still running. However, we agreed in the PRIDE workshop that we will remove the default pop-up messages from MagpieBridge. A PR for that is not created yet, but this will be eventually solved.

Therefore, this PR should now be ready for merge @sim642.

[sim642]

I realized that the analyze method was indeed itself blocking, and it had nothing to do with Magpie's threading #35.

It still does to the extent that if Magpie used an unbounded thread pool for its requests, then it would've worked all along since Magpie itself would have been able to immediately start all new requests in new threads without waiting for the old blocking ones. Although overall it's more reliable anyway to manage our own threads and not rely on internal implementation details of Magpie thread pools.