Ancient history: question about some mysterious code in MessageService.java

[Bryan Pendleton <bp...@gmail.com>]

I've been digging into o.a.d.iapi.services.i18n.MessageService.java.

Going all the way back to the original code import (revision 37083),
MessageService.getLocalizedMessage() contains this bit of code:

                 try {
                         msg[0] = formatMessage(getBundleForLocale(locale, messageId), messageId, arguments, true);
                         rc[0] = 0;
                         return;
                 } catch (MissingResourceException mre) {
                         // message does not exist in the requested locale
                         // most likely it does exist in our fake base class _en, so try that.
                 } catch (ShutdownException se) {
                 }
                 msg[0] = formatMessage(getBundleForLocale(EN, messageId), messageId, arguments, false);
                 rc[0] = 0;

I'm wondering specifically about the bit:

                 } catch (ShutdownException se) {
                 }

Why is this code here? Under what circumstances would we want to
discard the ShutdownException and continue processing like this?

Anybody have any memories of why this code was written this way?

I did spend some time reading DERBY-4920, DERBY-4911, and particularly
the epic DERBY-4741, all of which suggest that the presence of ShutdownException
handling in the code, in general, is due to the fact that when the engine
is being shutdown, there may be multiple threads which are active, and
the engine's shutdown logic uses JDK "interrupt" features to cause those
threads to receive ShutdownException exceptions being thrown.

But those old JIRAs, marvelous as they are, didn't really clarify why
MessageService has any business discarding the exception in this manner.

thanks,

bryan

Re: Ancient history: question about some mysterious code in MessageService.java

[Bryan Pendleton <bp...@gmail.com>]

Thanks Rick, for the pointers and ideas.

> In the code block which you are studying, the message formatting logic responds to the ShutdownException by falling through to its last chance handler. Do you think it should do something else?

No, I don't propose to change its behavior, just to understand it.

In the context of DERBY-6773, Abhinav and I have been studying whether
we can refactor MessageService.getLocalizedMessage() so that it
resides in the shared area of the code and can be bound into both
the client and server jars.

The intent of the refactoring is to gather all the code which knows
about packing exceptions into network messages (SQLERRMC blocks) and
then de-packing them on the other side into a single class, rather
than the current situation where the code is rather spread about.

And the code just above the code I quoted yesterday is directly
dependent on other code in the networking classes that we're also
proposing to refactor; it encodes message arguments with special delimiters:

			char [] sqlerrmc_chars = sqlerrmc.toCharArray();
			int numArgs = 0, lastSepIdx = -1; // last separator index
			for (int i = 0; i < sqlerrmc_chars.length; i++)
			{
				if (sqlerrmc_chars[i] == 20)	// separator
				{
					numArgs++;
					lastSepIdx = i;
				}
			}

That's the code I'd like to be able to call directly from the client libraries.

The dependence on ShutdownException is the only thing (that I can tell)
which is requiring these two methods to be in the engine jar; otherwise
they could clearly be in the shared code.

But to accomplish that, we'll also need to refactor ShutdownException
so that it is part of the shared code, too. Which is a tiny change, but
is still a widening of our proposal.

I was searching for ways to avoid it, e.g., by having getLocalizedMessage()
throw the ShutdownException and have its caller catch and handle it, but
that's not a 1-line change to the code, either.

I guess I was sort of hoping somebody might say:

     "Oh! That's dead code! It was in there as early scaffolding, and
     can simply be removed."

But I don't believe that to be the case, and will stop hoping for that.

  :)

bryan

Re: Ancient history: question about some mysterious code in MessageService.java

[Rick Hillegas <ri...@gmail.com>]

On 7/4/15 11:55 AM, Bryan Pendleton wrote:
>
> I've been digging into o.a.d.iapi.services.i18n.MessageService.java.
>
> Going all the way back to the original code import (revision 37083),
> MessageService.getLocalizedMessage() contains this bit of code:
>
>                 try {
>                         msg[0] = 
> formatMessage(getBundleForLocale(locale, messageId), messageId, 
> arguments, true);
>                         rc[0] = 0;
>                         return;
>                 } catch (MissingResourceException mre) {
>                         // message does not exist in the requested locale
>                         // most likely it does exist in our fake base 
> class _en, so try that.
>                 } catch (ShutdownException se) {
>                 }
>                 msg[0] = formatMessage(getBundleForLocale(EN, 
> messageId), messageId, arguments, false);
>                 rc[0] = 0;
>
> I'm wondering specifically about the bit:
>
>                 } catch (ShutdownException se) {
>                 }
>
> Why is this code here? Under what circumstances would we want to
> discard the ShutdownException and continue processing like this?
>
> Anybody have any memories of why this code was written this way?
>
> I did spend some time reading DERBY-4920, DERBY-4911, and particularly
> the epic DERBY-4741, all of which suggest that the presence of 
> ShutdownException
> handling in the code, in general, is due to the fact that when the engine
> is being shutdown, there may be multiple threads which are active, and
> the engine's shutdown logic uses JDK "interrupt" features to cause those
> threads to receive ShutdownException exceptions being thrown.
>
> But those old JIRAs, marvelous as they are, didn't really clarify why
> MessageService has any business discarding the exception in this manner.
>
> thanks,
>
> bryan
>
Hi Bryan,

I don't have any previous experience with how Derby handles 
ShutdownException. However, I've looked at all the places where we throw 
and catch this exception. My sense is that ShutdownException is raised 
when the engine is seriously confused about its internal state, chiefly 
while handling race conditions during (possibly user-initiated) 
shutdown. In almost all cases (including the one you're studying) the 
response to ShutdownException is to silently swallow it and then 
continue with what you were doing. There is one place where we report 
this exception--that only happens when the engine is up on the blocks, 
being examined while debugging a problem.

In the code block which you are studying, the message formatting logic 
responds to the ShutdownException by falling through to its last chance 
handler. Do you think it should do something else?

Thanks,
-Rick

--------------------------------------

Here are some notes I jotted down about how ShutdownException is used:


Thrown when context management is confused or interrupted:

     ContextManager.checkInterrupt()
     ContextService.getFactory()

Caught and swallowed:

     SystemContext.cleanupOnError()  // race conditions during shutdown. 
just continue shutting down.
     MessageService.getTextMessage() // fall through to last chance 
message formatting logic
     MessageService.getLocalizedMessage()  // fall through to last 
chance message formatting logic
     InterruptStatus.setInterrupted()  // continue interrupt processing
     InterruptStatus.restoreIntrFlagIfSeen()  // continue interrupt 
processing
     InterruptStatus.restoreIntrFlagIfSeen() // continue interrupt 
processing
     IndexStatisticsDaemonImpl.processingLoop() // continue thread shutdown
     IndexStatisticsDaemonImpl.stop() // continue thread shutdown
     LogToFile.performWork()  // continue winding down a checkpoint

Caught and recorded only if we are tracing while debugging a problem:

     IndexStatisticsDaemonImpl.run()

Caught and handled as a failure:

     BaseMonitor.getBundle()