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.

Sign up for GitHub

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

Jump to bottom

Removing Class.forName for implementation bridge helpers #28912

Merged
merged 12 commits into from
May 17, 2022
Merged

Removing Class.forName for implementation bridge helpers #28912

merged 12 commits into from
May 17, 2022

Conversation

FabianMeiswinkel
Copy link
Member

@FabianMeiswinkel FabianMeiswinkel commented May 17, 2022 •
edited
Loading

Description

This PR is making changes to the implementation bridge accessors to avoid a risk for hitting a dead-lock on static initialization (we have seen it in ComsosException - see below).

azure_cosmos_spark.com.azure.cosmos.CosmosException.<clinit>(CosmosException.java:547)
azure_cosmos_spark.com.azure.cosmos.implementation.directconnectivity.rntbd.RntbdRequestManager.messageReceived(RntbdRequestManager.java:868)
azure_cosmos_spark.com.azure.cosmos.implementation.directconnectivity.rntbd.RntbdRequestManager.channelRead(RntbdRequestManager.java:187)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
azure_cosmos_spark.io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:327)
azure_cosmos_spark.io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:299)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
azure_cosmos_spark.io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
azure_cosmos_spark.io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:253)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
azure_cosmos_spark.io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
azure_cosmos_spark.io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1371)
azure_cosmos_spark.io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1234)
azure_cosmos_spark.io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1283)
azure_cosmos_spark.io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:510)
azure_cosmos_spark.io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:449)
azure_cosmos_spark.io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:279)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
azure_cosmos_spark.io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
azure_cosmos_spark.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
azure_cosmos_spark.io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
azure_cosmos_spark.io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
azure_cosmos_spark.io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:722)
azure_cosmos_spark.io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:658)
azure_cosmos_spark.io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:584)
azure_cosmos_spark.io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:496)
azure_cosmos_spark.io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986)
azure_cosmos_spark.io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
azure_cosmos_spark.io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
java.lang.Thread.run(Thread.java:748)

The approach to avoid the possible deadl-lcok is two-fold:

  1. Avoid throwing exception in static initializers (on one critical failure - which would indicate an unrecoverable bug - we instead use system.exit)
  2. Don't use Class.forName - but instead use a static method to force class initialization
  3. Don't force class initialization in the setxxx-accessor method (because that one would only ever be called by the class that would need to be initialized - so it is already initialized at this point)

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@ghost ghost added the Cosmos label May 17, 2022
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-cosmos

xinlian12
xinlian12 approved these changes May 17, 2022
View reviewed changes
Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@FabianMeiswinkel FabianMeiswinkel merged commit 2a25c89 into Azure:main May 17, 2022
anushkasingh16 pushed a commit to anushkasingh16/azure-sdk-for-java that referenced this pull request Jun 6, 2022
@FabianMeiswinkel @anushkasingh16
Removing Class.forName for implementation bridge helpers (Azure#28912)
22de02f 
* improvement for CosmosExceptionHelper

* update log message

* Removing Class.forName for implementation bridge helpers

* Added changelog

* Update spotbugs-exclude.xml

* Update spotbugs-exclude.xml

* Adding API comments

* Update CHANGELOG.md

Co-authored-by: annie-mac <annie-mac@yindeng2019.fareast.corp.microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xinlian12 xinlian12 xinlian12 approved these changes

@kushagraThapar kushagraThapar kushagraThapar approved these changes

@kirankumarkolli kirankumarkolli Awaiting requested review from kirankumarkolli kirankumarkolli is a code owner

@mbhaskar mbhaskar Awaiting requested review from mbhaskar

@simplynaveen20 simplynaveen20 Awaiting requested review from simplynaveen20

@milismsft milismsft Awaiting requested review from milismsft

@aayush3011 aayush3011 Awaiting requested review from aayush3011

@mssfang mssfang Awaiting requested review from mssfang mssfang is a code owner

@JonathanGiles JonathanGiles Awaiting requested review from JonathanGiles JonathanGiles is a code owner

Assignees
No one assigned
Labels
Cosmos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@FabianMeiswinkel @azure-sdk @kushagraThapar @xinlian12