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

Inconsistent formatting when reading multiple XLS files concurrently #1469

Open
1 of 5 tasks
Vemahk opened this issue Jan 2, 2025 · 5 comments · May be fixed by #1470
Open
1 of 5 tasks

Inconsistent formatting when reading multiple XLS files concurrently #1469

Vemahk opened this issue Jan 2, 2025 · 5 comments · May be fixed by #1470
Milestone

Comments

@Vemahk
Copy link

Vemahk commented Jan 2, 2025

NPOI Version

2.7.2

File Type

  • XLSX
  • XLS
  • DOCX
  • XLSM
  • OTHER

Reproduce Steps

Reproducible code + spreadsheet:
Demo.zip

Excel File only:
test.xls

Create a XLS spreadsheet with a date+time column. Copy the date time column one over. Format the first column to show only dates, and format the second to show only times. Then, in two different threads, open the spreadsheet and parse with NPOI.

This is done in the above Demo.zip

Issue Description

When reading the same XLS file (or two similar ones) on different threads, cell values are inconsistently formatted when read using DataFormatter.FormatCellValue(ICell). I believe this bug was introduced with 96186d6. This was fixed in upstream POI with apache/poi@00c69b3.

@Vemahk Vemahk added the bug label Jan 2, 2025
@Bykiev
Copy link
Collaborator

Bykiev commented Jan 3, 2025

Hi, nice catch, would you like to contribute?

@Bykiev Bykiev added the xls label Jan 3, 2025
@tonyqus tonyqus added this to the NPOI 2.7.3 milestone Jan 4, 2025
@Vemahk
Copy link
Author

Vemahk commented Jan 4, 2025

Yeah I can take a crack at it. Made the initial bug report as part of my day job, but I got some free time and remember my way around. PR heading y'all's way in a little bit.

@Vemahk
Copy link
Author

Vemahk commented Jan 4, 2025

I'm out of time for tonight, but I at least got a test case, benchmark, and the "simple solution" done. I want to still go back and fix, or at least reduce, the performance regression when I have some more time. Or someone else can pick it up from there if I never get the chance.

@Bykiev
Copy link
Collaborator

Bykiev commented Jan 4, 2025

I thought you'll port the changes with a ThreadLocal from apache/poi@00c69b3

@Vemahk
Copy link
Author

Vemahk commented Jan 5, 2025

If I get some time today, that's my goal. I've never used ThreadLocal or AsyncLocal in C#, though, so I wanted to spend more time figuring it out and making sure it worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants