Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OOM error handling, Fixes AB#3124149 #2567

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

fadidurah
Copy link
Contributor

@fadidurah fadidurah commented Jan 10, 2025

Update our error handling for the out of memory exception. Currently, we return out of memory exception like any other exception caught through our exception adapter. In the case of out of memory exception, we should shut down broker process gracefully, to allow reallocation of free memory for the broker process. Calling application will fail the request that triggered the out of memory exception and cause broker process to end. Next request by calling application will launch a new broker process.

Validation:

  • Tested will MsalTestApp + BrokerHost. Was able to call System.Exit() during exception handling and was able to shut down broker process without causing calling app (MsalTestApp) to crash.
  • Tested stack trace attribute by triggering a null pointer exception in code, and create a string representation of stack trace from the exception.

AB#3124149

@fadidurah fadidurah requested a review from mohitc1 January 10, 2025 04:28
@fadidurah fadidurah requested a review from a team as a code owner January 10, 2025 04:28
Copy link

✅ Work item link check complete. Description contains link AB#3124149 to an Azure Boards work item.

1 similar comment
Copy link

✅ Work item link check complete. Description contains link AB#3124149 to an Azure Boards work item.

@github-actions github-actions bot changed the title Fix OOM error handling Fix OOM error handling, Fixes AB#3124149 Jan 10, 2025
// Shut down current process
// Calling client app will receive "killed unexpectedly" error from MSAL if broker is shut down with this statement
// Calling application should not crash, and should be able to make another call, which will result in a new broker process.
System.exit(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What error/response does calling app receive in when this happens?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also android.os.Process.killProcess(android.os.Process.myPid()); this might be better to use in case of OOM. I read a bit, you may also read once and we can finalize

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants