ClickHouse / ClickHouse/clickhouse-java
DataTypeConverter usage has potential thread-safety issues
- Langage dominant
- Java
- Étoiles
- 1.6k
- Forks
- 636
- Merge moyen
- 2 j 23 h
- PR mergées (30 j)
- 29
Description
## Description
_This is not an explicit bug report in the sense that I haven't experienced a problem (yet?), it is just something I bumped into while browsing the code_.
`DataTypeConverter` has a [javadoc comment stating that it is not thread safe](https://github.com/ClickHouse/clickhouse-java/blob/main/client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java#L25). For what it's worth, I have also confirmed thread safety problems because I ended up using the `DataTypeConverter.INSTANCE` in production (before I realised that it was labelled as non thread safe) which caused some bugs.
After fixing the bugs in my project, I was curious about the fact that a static singleton `DataTypeConverter.INSTANCE` was even declared in the `clickhouse-java` repo (why have a singleton instance if it is not thread safe, in an otherwise thread safe library?). Of course this class is in a package named `internal`, so it's not really my business... :) Still, I did notice that `DataTypeConverter.INSTANCE` is used in [two](https://github.com/ClickHouse/clickhouse-java/blob/8da0b0c9aa7995b9fa839b56861a28047f71076a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java#L48) [places](https://github.com/ClickHouse/clickhouse-java/blob/8da0b0c9aa7995b9fa839b56861a28047f71076a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java#L90) which makes me wonder if it could be the source of (other / future / not-yet-encountered) problems..
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
Commencez par le javadoc et l’implémentation de client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java, puis examinez ses utilisations dans MapBackedRecord.java et AbstractBinaryFormatReader.java. Suivez l’état mutable et les tests existants afin d’établir le contrat de sécurité des threads prévu ; le travail est terminé lorsqu’il inclut une couverture qui exerce l’INSTANCE partagée de manière sûre ou documente pourquoi elle ne peut pas être partagée.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- java
- Domaine
- api, backend
- Type d'issue
- Bug
- Difficulté
- 4/5
- Temps estimé
- 3-5 jours
- Activité
- Calme
- Clarté
- À clarifier
- Accessibilité débutants
- 38/100