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

[Go] Fix segfault on string tensors with mismatched dimensions #50508

Merged
merged 1 commit into from Jul 13, 2021

Conversation

wamuir
Copy link
Contributor

@wamuir wamuir commented Jun 29, 2021

This PR fixes a segmentation violation that may occur during GC on string tensors. The segfault results in a flaky test for TestNewTensor at the error test for mismatched dimensions for strings

// Mismatched dimensions for strings
[][]string{{"abc"}, {"abcd", "abcd"}},

For string tensors, C.TF_TString_Dealloc is called during garbage collection within a finalizer function. However, tensor structure isn't checked until encoding to avoid a performance penalty. The current method for dealloc assumes that encoding succeeded, but segfaults when a string tensor is garbage collected whose encoding failed (e.g., due to mismatched dimensions).

To fix this, the call to set the finalizer function is deferred until NewTensor returns and, if encoding failed for a string tensor, deallocs are determined based on bytes written.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Jun 29, 2021
@google-cla google-cla bot added the cla: yes label Jun 29, 2021
@gbaned gbaned self-assigned this Jun 29, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jun 29, 2021
@gbaned gbaned added the awaiting review Pull request awaiting review label Jul 2, 2021
@gbaned gbaned requested a review from gharibian July 8, 2021 12:38
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 9, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 9, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 9, 2021
@gbaned gbaned removed the awaiting review Pull request awaiting review label Jul 12, 2021
@rthadur rthadur added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Jul 12, 2021
@gbaned gbaned removed the ready to pull PR ready for merge process label Jul 13, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 13, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 13, 2021
@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Jul 13, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 13, 2021
@copybara-service copybara-service bot merged commit 8721ba9 into tensorflow:master Jul 13, 2021
16 checks passed
PR Queue automation moved this from Approved by Reviewer to Merged Jul 13, 2021
Subrahmanyajoshi added a commit to Subrahmanyajoshi/Cancer-Detection-using-GCP that referenced this pull request Sep 9, 2021
1. Updated tensorflow version to 2.6.0 as all versions below 2.5.1 seem to have a security vulnerability as explained here -> tensorflow/tensorflow#50508
2. Updated run time version from 2.3 to 2.5 in ai platform trainer notebook as runtime version of 2.6 currently is not supported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants