Skip to content

Guard NULL from GetStringUTFChars in raiseTypeError - #646

Open
SAY-5 wants to merge 2 commits into
ninia:dev_4.3from
SAY-5:fix-null-utfchars-raisetypeerror
Open

Guard NULL from GetStringUTFChars in raiseTypeError#646
SAY-5 wants to merge 2 commits into
ninia:dev_4.3from
SAY-5:fix-null-utfchars-raisetypeerror

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jul 20, 2026

Copy link
Copy Markdown

Fixes #643. In raiseTypeError, the return value of GetStringUTFChars was passed straight to PyErr_Format as a %s argument. Per the JNI spec that call returns NULL if it can't allocate the native string, and formatting NULL with %s crashes. This falls back to "" for the message and skips the matching ReleaseStringUTFChars when nothing was returned, so the type-error path stays intact on that rare allocation failure.

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@bsteffensmeier

Copy link
Copy Markdown
Member

It appears there are 3 possible failure modes a JVM could use when GetStringUTFChars fails.

  1. It can exit the JVM or crash
  2. It can return null with no exception
  3. It can return null and raise an OutOfMemoryError

There is nothing we can do in the first case. This fix is good for the second case. In the third case having a java exception set is likely to cause problems later. Since this new code handles the exception I think we need to add a call to clear the JNI exception.

…Error

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@SAY-5

SAY-5 commented Jul 22, 2026

Copy link
Copy Markdown
Author

Good catch on the OutOfMemoryError case. I added an ExceptionClear right after the failed GetStringUTFChars so the pending JNI exception doesn't leak into later calls; the fallback name still gets reported.

@bsteffensmeier bsteffensmeier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, thanks

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.

3 participants