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 null pointer exception when importing program members multithread. #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pn-koshikawa
Copy link

Are all connections created by the plugin secure?

  • Does it opt secure communication standard? Such as HTTPS, SSH, SFTP, SMTP STARTTLS. If not check with CISO to decide we can deploy the plugin.
  • Does support both authentication and encryption appropriately? Such as: "just encrypting without authentication" that is insecure.

Does the plugin connect only to its expected external site which the customer explicitly set in their config file?

  • Does NOT connect unexpected external site and our internal endpoints? Such as: “v3/job/:id/set_started” callback endpoint.

Does NOT the plugin persist any customers' private information? Identify the private information beforehand.

  • Does NOT include them in (temporary) files?
  • Does NOT include them in log messages and exception messages?

What kind of environments does the plugin interact with?

  • Does NOT execute any shell command?
  • Does NOT read any files on the running instance? Such as: "/etc/passwords". It’s ok to read temporary files that the plugin wrote.
  • Does use to create temporary files by spi.TempFileSpace utility to avoid the conflict of the file names.
  • Does NOT get environment variables or JVM system properties at runtime? Such as AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY in environment variables

Does NOT the plugin use insecure libraries?

  • Line up all depending library so that we can identify the impact of security incident of those library if any.
  • Check libraries usage of the plugin; all security check list must apply to the library usages. Such as "Are all connections created by the library secure?"

Does NOT the plugin source code repository contain kinds of credentials

  • API keys
  • Passwords

Make sure to free up all resources allocated during Embulk transaction “committing” or “rolling back”or before.

  • Network (connections, pooled connections)
  • Memory (cache in static variables)
  • File (temporary files)
  • CPU (threads, processes)

@pn-koshikawa
Copy link
Author

pn-koshikawa commented Jan 23, 2024

Hi, I encountered a null pointer exception while importing program members in a multi-threaded execution context.

I'm wondering if PageBuilder in Embulk is not designed to be thread-safe. I'm referring to the code in the PageBuilder class, specifically where a buffer is flushed when exceeded by addRecord. If the buffer size is exceeded during this flush, a bufferSize exception is thrown.

Here are the relevant code snippets from the Embulk GitHub repository:

PageBuilder.java (lines 183-200)
PageBuilder.java (line 213)
In a multi-threaded scenario, is there a possibility that the timing of writing to bufferSlice could be null, or that multiple threads might attempt to write to bufferSlice simultaneously, leading to this null pointer exception?

@pn-koshikawa pn-koshikawa marked this pull request as ready for review January 23, 2024 05:52
@pn-koshikawa pn-koshikawa requested a review from a team as a code owner January 23, 2024 05:52
@pn-koshikawa
Copy link
Author

@xuantuan58 @luongnhattruong, I would greatly appreciate it if either of you could review my code. Thanks in advance for your help!

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.

1 participant