Remove abort when catching a signal. Combined with signal handler cha…

[wjsl]

…ining from the JVM, this should be prevent the JVM from shutting down when exception_handler gets called.

[hillu]

Mhm. Why does the JVM produce SIGBUS or SIGSEGV?

[wjsl]

See this. SIGBUS and SIGSEGV are used internally for generating things like NullPointerExceptions. They're intercepted here and the entire thing aborts.

I have a test case that involves reloading Yara rules on the fly and scanning some byte arrays. After upgrading to Yara 3.5.0 and newer, I had to do some hacking to get things to work. Part of the puzzle was removing the assertion here, the other part was using libjsig.so via LD_PRELOAD to allow the JVM to correctly route the signals.

[hillu]

@wjsl Do you think that it would make sense for exception_handler to call the previously-set signal handler instead?

[wjsl]

It could. What would it look like?

Before I noticed the test case, I hacked up a little program that simulates how many threads I'm using that can be found here. Using gdb I can set a breakpoint in exception.h and find myself in the exception handler once that Segfaulter function runs.

Even from a non-Java environment, what I think the issue is that a signal handler is really set for just the process, but the signal handler assumes the signal is delivered to the thread that caused the signal in the first place.

I'm starting to think that removing the assert(false) is not what I want. A work around for me could be to scan with SCAN_FLAGS_NO_TRYCATCH turned on.

[plusvic]

I had the same problem with go-yara recently. A nil-pointer dereference caused a SIGSEGV in some other portion of my code totally unrelated to YARA, but the signal was captured by YARA's handler and reached the assert statement. The assertion caused a SIGABRT and as a result Go coredumped the process as expected, but the original cause of the problem was hidden. It wasn't until I changed go-yara to use SCAN_FLAGS_NO_TRYCATCH that I discovered that the issue wasn't related to YARA or go-yara at all.

So I'm starting to wonder if it would be better to let the application install the signal handler if needed, or at least make SCAN_FLAGS_NO_TRYCATCH the default option. The signal handler can be problematic when using YARA as a library within a larger program.

[hillu]

@plusvic I agree that having the default signal handler may be problematic. And I think we all agree that the abort() at the end of the signal handler does more harm than good, so let's at least remove that, as suggested by @wjsl.

I wonder if chaining the signal handler (as laid out in https://github.com/hillu/yara/tree/chained-signal-handler) might be a better general answer.

[plusvic]

I would do both, setting SCAN_FLAGS_NO_TRYCATCH as the default and chaining signal handlers. I have some questions about https://github.com/hillu/yara/tree/chained-signal-handler, I'll comment there.

[wjsl]

Thanks for the merge! I didn't realize there was a whole other discussion under another PR. Apologies for the radio silence.